On Fri, Oct 27, 2017 at 01:50:26AM +0200, Jan Engelhardt wrote: > #include <openssl/des.h> > int main(void) { > char salt[3] = {0xf8, 0xd0, 0x00}; > char out[32]; > DES_fcrypt("foo", salt, out); > }
This program produces a Segmentation fault in OpenBSD current. > openssl 1.1.x has it fixed (but 1.0.2l does not!) - their commit > seems to be 6493e4801e9edbe1ad1e256d4ce9cd55c8aa2242 in > https://github.com/openssl/openssl . Their fix changes the semantics of DES_crypt() and DES_fcrypt(). They may fail and return NULL now. This was never the case before so we may expect that programs do not check it. With DES_fcrypt() it is very likely that some uninitilaized content of the return string is used. So I have extended the workaround that was there already. Read the comment above the fix. If the salt does not consist of two ascii characters, replace it with "AA". This gives a safe result in all cases. ok? bluhm Index: lib/libcrypto/des/fcrypt.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/lib/libcrypto/des/fcrypt.c,v retrieving revision 1.12 diff -u -p -r1.12 fcrypt.c --- lib/libcrypto/des/fcrypt.c 26 Dec 2016 21:30:10 -0000 1.12 +++ lib/libcrypto/des/fcrypt.c 5 Dec 2017 16:03:57 -0000 @@ -78,9 +78,15 @@ char *DES_fcrypt(const char *buf, const * crypt to "*". This was found when replacing the crypt in * our shared libraries. People found that the disabled * accounts effectively had no passwd :-(. */ - x=ret[0]=((salt[0] == '\0')?'A':salt[0]); + x = salt[0]; + if (x == 0 || x >= sizeof(con_salt)) + x = 'A'; + ret[0] = x; Eswap0=con_salt[x]<<2; - x=ret[1]=((salt[1] == '\0')?'A':salt[1]); + x = (salt[0] == '\0') ? 'A' : salt[1]; + if (x == 0 || x >= sizeof(con_salt)) + x = 'A'; + ret[1] = x; Eswap1=con_salt[x]<<6; /* EAY r=strlen(buf); Index: regress/lib/libcrypto/des/destest.c =================================================================== RCS file: /data/mirror/openbsd/cvs/src/regress/lib/libcrypto/des/destest.c,v retrieving revision 1.3 diff -u -p -r1.3 destest.c --- regress/lib/libcrypto/des/destest.c 30 Oct 2015 15:58:40 -0000 1.3 +++ regress/lib/libcrypto/des/destest.c 5 Dec 2017 16:02:18 -0000 @@ -749,18 +749,38 @@ plain[8+4], plain[8+5], plain[8+6], plai } 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", "\202B"); + if (strncmp("AB",str,2) != 0) { + printf("salt %s first non ascii not replaced with A\n", str); + err = 1; + } + str = DES_crypt("testing", "B\202"); + if (strncmp("BA",str,2) != 0) { + printf("salt %s second non ascii not replaced with A\n", str); + err = 1; + } + str = DES_crypt("testing", "\0B"); + if (strncmp("AA",str,2) != 0) { + printf("salt %s first NUL not replaced with AA\n", str); + err = 1; + } + str = DES_crypt("testing", "B"); + if (strncmp("BA",str,2) != 0) { + printf("salt %s second NUL not replaced with A\n", str); + err = 1; + } printf("\n"); return(err); }