diff options
author | Zack Weinberg <zackw@panix.com> | 2018-07-23 17:33:12 -0400 |
---|---|---|
committer | Björn Esser <besser82@fedoraproject.org> | 2018-07-26 11:24:37 +0200 |
commit | aae4c1baea534d2e4c9dfe2faf42ee0c5f7a6f22 (patch) | |
tree | 233c3a802d9b3311d731489e0f54b149092093af /Makefile.am | |
parent | 0c3ea90e14cf00023885183af7f401ce61f7a60f (diff) | |
download | libxcrypt-aae4c1baea534d2e4c9dfe2faf42ee0c5f7a6f22.tar.gz libxcrypt-aae4c1baea534d2e4c9dfe2faf42ee0c5f7a6f22.tar.bz2 libxcrypt-aae4c1baea534d2e4c9dfe2faf42ee0c5f7a6f22.zip |
Issue #15: predictable behavior when crypt args are NULL or invalid.
This patch introduces the following semantics for NULL or invalid
`phrase` and `setting` arguments to the `crypt*` functions:
- If `setting` is NULL, all of the functions will fail (in their
usual manner) with errno set to EINVAL, regardless of what `phrase`
is. (For crypt_r/rn/ra, problems with the `data` argument may be
detected first.)
- If `setting` is a valid string (in the sense that calling `strlen`
on it will not crash the program), but `phrase` is NULL, again all
of the functions will fail with errno set to EINVAL.
- If both `setting` and `phrase` are non-NULL, and one of them is an
_invalid_ string (in the sense that calling `strlen` on it _will_
crash the program), then the program will crash. The crash is
guaranteed to happen before any code from a hashing method is executed.
- If `phrase` is NULL and `setting` is an invalid string, then it is
unspecified whether the function will fail or whether the program
will crash, but it will be one of those two things, and, again, no
code from a hashing method will be executed. (The actual behavior
right now is that the function will fail *unless* `setting` is
non-NULL and there are fewer than two readable bytes at that memory
location, but I don't want to make that a contract.)
I would somewhat prefer the simpler semantics of always crashing on
NULL as well as on invalid strings, but there was some sentiment in
favor of returning failure for NULL, so I've done this to the maximum
extent that is practical. The complicated behavior when `phrase` is
NULL but `setting` is invalid is not practical to avoid without
breaking an existing guarantee, viz. that a "failure token" never
compares equal to the `setting` string, no matter what; I think that
guarantee is significantly more important than corner-case behavior
like this.
Note also that we have __nonnull annotations on all the pointer
arguments to all these functions, which means the C compiler
unconditionally treats passing NULL as any of those arguments as
provoking undefined behavior; I actually had to make crypt-port.h
suppress those annotations when building the library, in order to
get the tests to pass.
Diffstat (limited to 'Makefile.am')
-rw-r--r-- | Makefile.am | 3 |
1 files changed, 2 insertions, 1 deletions
diff --git a/Makefile.am b/Makefile.am index f5e8df4..201dea5 100644 --- a/Makefile.am +++ b/Makefile.am @@ -148,7 +148,7 @@ check_PROGRAMS = \ test-crypt-pbkdf1-sha1 test-crypt-sha256 test-crypt-sha512 \ test-crypt-sunmd5 \ test-byteorder test-badsalt test-badsetting test-gensalt \ - test-short-outbuf \ + test-crypt-badargs test-short-outbuf \ test-getrandom-interface test-getrandom-fallbacks if ENABLE_OBSOLETE_API @@ -193,6 +193,7 @@ test_badsetting_LDADD = libcrypt.la test_gensalt_LDADD = libcrypt.la test_des_obsolete_LDADD = libcrypt.la test_des_obsolete_r_LDADD = libcrypt.la +test_crypt_badargs_LDADD = libcrypt.la test_short_outbuf_LDADD = libcrypt.la # These tests call internal APIs that may not be accessible from the |