From 6493e4801e9edbe1ad1e256d4ce9cd55c8aa2242 Mon Sep 17 00:00:00 2001 From: Rich Salz Date: Tue, 31 May 2016 23:05:48 -0400 Subject: [PATCH] RT4337: Crash in DES Salt must be two ASCII characters. Add tests to check for that, and a test to test the checks. Reviewed-by: Matt Caswell --- crypto/des/fcrypt.c | 51 ++++++++++++++++----------------------------- doc/crypto/des.pod | 17 +++++++++------ test/destest.c | 21 +++++++++++++++---- 3 files changed, 46 insertions(+), 43 deletions(-) diff --git a/crypto/des/fcrypt.c b/crypto/des/fcrypt.c index b52f486acb..5215ad3e64 100644 --- a/crypto/des/fcrypt.c +++ b/crypto/des/fcrypt.c @@ -66,27 +66,23 @@ char *DES_crypt(const char *buf, const char *salt) char e_buf[32 + 1]; /* replace 32 by 8 ? */ char *ret; - /* Copy at most 2 chars of salt */ - if ((e_salt[0] = salt[0]) != '\0') - e_salt[1] = salt[1]; + if (salt[0] == '\0' || salt[1] == '\0') + return NULL; - /* Copy at most 32 chars of password */ - strncpy(e_buf, buf, sizeof(e_buf)); + /* Copy salt, convert to ASCII. */ + e_salt[0] = salt[0]; + e_salt[1] = salt[1]; + e_salt[2] = '\0'; + ebcdic2ascii(e_salt, e_salt, sizeof(e_salt)); - /* Make sure we have a delimiter */ - e_salt[sizeof(e_salt) - 1] = e_buf[sizeof(e_buf) - 1] = '\0'; - - /* Convert the e_salt to ASCII, as that's what DES_fcrypt works on */ - ebcdic2ascii(e_salt, e_salt, sizeof e_salt); - - /* Convert the cleartext password to ASCII */ + /* Convert password to ASCII. */ + OPENSSL_strlcpy(e_buf, buf, sizeof(e_buf)); ebcdic2ascii(e_buf, e_buf, sizeof e_buf); - /* Encrypt it (from/to ASCII) */ + /* Encrypt it (from/to ASCII); if it worked, convert back. */ ret = DES_fcrypt(e_buf, e_salt, buff); - - /* Convert the result back to EBCDIC */ - ascii2ebcdic(ret, ret, strlen(ret)); + if (ret != NULL) + ascii2ebcdic(ret, ret, strlen(ret)); return ret; #endif @@ -103,25 +99,14 @@ char *DES_fcrypt(const char *buf, const char *salt, char *ret) unsigned char *b = bb; unsigned char c, u; - /* - * eay 25/08/92 If you call crypt("pwd","*") as often happens when you - * have * as the pwd field in /etc/passwd, the function returns - * *\0XXXXXXXXX The \0 makes the string look like * so the pwd "*" would - * crypt to "*". This was found when replacing the crypt in our shared - * libraries. People found that the disabled accounts effectively had no - * passwd :-(. - */ -#ifndef CHARSET_EBCDIC - x = ret[0] = ((salt[0] == '\0') ? 'A' : salt[0]); + x = ret[0] = salt[0]; + if (x == 0 || x >= sizeof(con_salt)) + return NULL; Eswap0 = con_salt[x] << 2; - x = ret[1] = ((salt[1] == '\0') ? 'A' : salt[1]); + x = ret[1] = salt[1]; + if (x == 0 || x >= sizeof(con_salt)) + return NULL; Eswap1 = con_salt[x] << 6; -#else - x = ret[0] = ((salt[0] == '\0') ? os_toascii['A'] : salt[0]); - Eswap0 = con_salt[x] << 2; - x = ret[1] = ((salt[1] == '\0') ? os_toascii['A'] : salt[1]); - Eswap1 = con_salt[x] << 6; -#endif /* * EAY r=strlen(buf); r=(r+7)/8; diff --git a/doc/crypto/des.pod b/doc/crypto/des.pod index 7ccadbc345..0131093ba9 100644 --- a/doc/crypto/des.pod +++ b/doc/crypto/des.pod @@ -240,8 +240,9 @@ is thread safe, unlike the normal crypt. DES_crypt() is a faster replacement for the normal system crypt(). This function calls DES_fcrypt() with a static array passed as the -third parameter. This emulates the normal non-thread safe semantics +third parameter. This mostly emulates the normal non-thread-safe semantics of crypt(3). +The B must be two ASCII characters. DES_enc_write() writes I bytes to file descriptor I from buffer I. The data is encrypted via I (default) @@ -272,15 +273,11 @@ DES_string_to_key() is available for backward compatibility with the MIT library. New applications should use a cryptographic hash function. The same applies for DES_string_to_2key(). -=head1 CONFORMING TO - -ANSI X3.106 +=head1 NOTES The B library was written to be source code compatible with the MIT Kerberos library. -=head1 NOTES - Applications should use the higher level functions L etc. instead of calling these functions directly. @@ -288,6 +285,14 @@ functions directly. Single-key DES is insecure due to its short key size. ECB mode is not suitable for most applications; see L. +=head1 HISTORY + +The requirement that the B parameter to DES_crypt() and DES_fcrypt() +be two ASCII characters was first enforced in +OpenSSL 1.1.0. Previous versions tried to use the letter uppercase B +if both character were not present, and could crash when given non-ASCII +on some platforms. + =head1 SEE ALSO L, diff --git a/test/destest.c b/test/destest.c index 389d0c8352..877f71d3fb 100644 --- a/test/destest.c +++ b/test/destest.c @@ -35,8 +35,6 @@ int main(int argc, char *argv[]) #else # include -# define crypt(c,s) (DES_crypt((c),(s))) - /* tisk tisk - the test keys don't all have odd parity :-( */ /* test data */ # define NUM_TESTS 34 @@ -660,16 +658,31 @@ int main(int argc, char *argv[]) } printf("\n"); printf("fast crypt test "); - str = crypt("testing", "ef"); + str = DES_crypt("testing", "ef"); if (strcmp("efGnQx2725bI2", str) != 0) { printf("fast crypt error, %s should be efGnQx2725bI2\n", str); err = 1; } - str = crypt("bca76;23", "yA"); + str = DES_crypt("bca76;23", "yA"); if (strcmp("yA1Rp/1hZXIJk", str) != 0) { printf("fast crypt error, %s should be yA1Rp/1hZXIJk\n", str); err = 1; } + str = DES_crypt("testing", "y\202"); + if (str != NULL) { + printf("salt error only usascii are accepted\n"); + err = 1; + } + str = DES_crypt("testing", "\0A"); + if (str != NULL) { + printf("salt error cannot contain null terminator\n"); + err = 1; + } + str = DES_crypt("testing", "A"); + if (str != NULL) { + printf("salt error must be at least 2\n"); + err = 1; + } printf("\n"); return (err); } -- 2.25.1