Hello All, I am not sure that the patches below correct any potential security issue, but use of values returned from calloc()/malloc() and alloca() without checking for NULL may result in undesirable behavior in OpenSSL 1.0.1c. The patches below result in a clean './config' and 'make' under CentOS 6.3 (64-bit).
In reviewing OpenSSL-1.0.1c, I found two instances where calloc() is called without a check for a return value of NULL which indicates failure. I found these calls in directory 'openssl-1.0.1c/crypto/engine', file 'eng_cryptodev.c'. The patch to add the necessary checks is below: --- eng_cryptodev.c.orig 2012-12-16 13:41:49.948556585 -0800 +++ eng_cryptodev.c 2012-12-16 13:46:14.780548132 -0800 @@ -1001,11 +1001,19 @@ if (r) { kop->crk_param[kop->crk_iparams].crp_p = calloc(rlen, sizeof(char)); + if (kop->crk_param[kop->crk_iparams].crp_p == NULL) { + printf("cryptodev_asym: Can't allocate memory via calloc() \n"); + return (ret); + } kop->crk_param[kop->crk_iparams].crp_nbits = rlen * 8; kop->crk_oparams++; } if (s) { kop->crk_param[kop->crk_iparams+1].crp_p = calloc(slen, sizeof(char)); + if (kop->crk_param[kop->crk_iparams+1].crp_p == NULL) { + printf("cryptodev_asym: Can't allocate memory via calloc() \n"); + return (ret); + } kop->crk_param[kop->crk_iparams+1].crp_nbits = slen * 8; kop->crk_oparams++; } Here is the object file produced by 'make': -rw-r--r--. 1 root root 1240 Dec 16 18:14 eng_cryptodev.o In directory 'openssl-1.0.1c/apps', file 'speed.c', I found an instance of malloc() without a check for a return value of NULL indicating failure. The patch file below adds the check for the return value from malloc(): --- speed.c.orig 2012-12-16 14:08:21.308630505 -0800 +++ speed.c 2012-12-16 14:10:07.472567659 -0800 @@ -2655,6 +2655,11 @@ static char sep[]=":"; fds=malloc(multi*sizeof *fds); + if (fds == NULL) + { + fprintf(stderr, "do_multi: unable to allocate memory\n"); + exit(1); + } for(n=0 ; n < multi ; ++n) { if (pipe(fd) == -1) Here is the object file produced by 'make': -rw-r--r--. 1 root root 116096 Dec 16 18:16 speed.o In directory 'openssl-1.0.1c/crypto/pkcs7', file 'example.c', I found a number of instances where malloc() was called, but no check for a return value of NULL was made, indicating failure. The patch file below adds the necessary checks to all malloc() calls which lack such checks: --- example.c.orig 2012-12-16 14:16:02.150623343 -0800 +++ example.c 2012-12-16 14:20:50.590549277 -0800 @@ -95,6 +95,8 @@ total=ASN1_object_size(1,i,V_ASN1_SEQUENCE); data=malloc(total); + if (data == NULL) + return 0; /* unable to allocate memory, leave */ p=data; ASN1_put_object(&p,1,i,V_ASN1_SEQUENCE,V_ASN1_UNIVERSAL); i2d_ASN1_OCTET_STRING(os1,&p); @@ -147,6 +149,8 @@ if (!asn1_const_Finish(&c)) goto err; *str1=malloc(os1->length+1); *str2=malloc(os2->length+1); + if ((*str1 == NULL) || (*str2 == NULL)) + goto err; /* unable to allocate memory, error */ memcpy(*str1,os1->data,os1->length); memcpy(*str2,os2->data,os2->length); (*str1)[os1->length]='\0'; @@ -259,6 +263,8 @@ total=ASN1_object_size(1,i,V_ASN1_SEQUENCE); data=malloc(total); + if (data == NULL) + return 0; /* unable to allocate memory */ p=data; ASN1_put_object(&p,1,i,V_ASN1_SEQUENCE,V_ASN1_UNIVERSAL); i2d_ASN1_OCTET_STRING(os1,&p); @@ -314,6 +320,8 @@ if (!asn1_const_Finish(&c)) goto err; *str1=malloc(os1->length+1); *str2=malloc(os2->length+1); + if ((*str1 == NULL) || (*str2 == NULL)) + goto err; /* unable to allocate memory, error */ memcpy(*str1,os1->data,os1->length); memcpy(*str2,os2->data,os2->length); (*str1)[os1->length]='\0'; In directory 'openssl-1.0.1c/crypto', file 'cryptlib.c', I found an instance of alloca() being called without a return value check being performed. A value of NULL being returned would mean that there is not enough stack space available left to properly complete the call. The patch file is below: --- cryptlib.c.orig 2012-12-16 16:37:54.969527776 -0800 +++ cryptlib.c 2012-12-16 16:38:59.718553187 -0800 @@ -811,6 +811,7 @@ if (len>512) return -1; /* paranoia */ len++,len&=~1; /* paranoia */ name=(WCHAR *)alloca(len+sizeof(WCHAR)); + if (name == NULL) return -1; /* no space on stack, paranoia */ if (!GetUserObjectInformationW (h,UOI_NAME,name,len,&len)) return -1; Here is the object file produced by 'make': -rw-r--r--. 1 root root 14104 Dec 16 18:14 cryptlib.o In directory 'openssl-1.0.1c/crypto/bn', file 'bn_exp.c', I found an instance of alloca() being called without a return value check being performed. A value of NULL being returned would mean that there is not enough stack space available left to properly complete the call. The patch file is below: --- bn_exp.c.orig 2012-12-16 16:47:56.863559052 -0800 +++ bn_exp.c 2012-12-16 16:49:05.213512656 -0800 @@ -634,6 +634,7 @@ #ifdef alloca if (powerbufLen < 3072) powerbufFree = alloca(powerbufLen+MOD_EXP_CTIME_MIN_CACHE_LINE_WIDTH); + if (powerbufFree == NULL) goto err; /* if NULL, not enough memory on stack */ else #endif if ((powerbufFree=(unsigned char*)OPENSSL_malloc(powerbufLen+MOD_EXP_CTIME_MIN_CACHE_LINE_WIDTH)) == NULL) Here is the object file produced by 'make': -rw-r--r--. 1 root root 20352 Dec 16 18:14 bn_exp.o In directory 'openssl-1.0.1c/crypto/bn', file 'bn_asm.c', I found two instances of alloca() being called without a return value check being performed. A value of NULL being returned would mean that there is not enough stack space available left to properly complete the call. The patch file is below: --- bn_asm.c.orig 2012-12-16 16:54:36.209532523 -0800 +++ bn_asm.c 2012-12-16 17:00:25.284555278 -0800 @@ -857,6 +857,8 @@ if (ap==bp) return bn_sqr_mont(rp,ap,np,n0p,num); #endif vp = tp = alloca((num+2)*sizeof(BN_ULONG)); + if (tp == NULL) /* if NULL, insufficient stack space available */ + return 1; /* Should we just return here, or is more needed? */ n0 = *n0p; @@ -990,6 +992,8 @@ int i=0,j; vp = tp = alloca((num+2)*sizeof(BN_ULONG)); + if (tp == NULL) /* if NULL, insufficient stack space available */ + return 1; /* Should we just return here, or is more needed? */ for(i=0;i<=num;i++) tp[i]=0; In directory 'openssl-1.0.1c/engines', file 'e_padlock.c', I found an instance of alloca() being called without a return value check being performed. A value of NULL being returned would mean that there is not enough stack space available left to properly complete the call. The patch file is below: --- e_padlock.c.orig 2012-12-16 17:03:55.025582911 -0800 +++ e_padlock.c 2012-12-16 17:08:03.397515021 -0800 @@ -1000,7 +1000,10 @@ /* optmize for small input */ allocated = (chunk<nbytes?PADLOCK_CHUNK:nbytes); out = alloca(0x10 + allocated); - out = NEAREST_ALIGNED(out); + if (out == NULL) /* if NULL, insufficient stack space available */ + out = out_arg; /* should we just set this to out_arg? */ + else + out = NEAREST_ALIGNED(out); } else out = out_arg; Here is the object file produced by 'make': -rw-r--r--. 1 root root 1240 Dec 16 18:16 e_padlock.o Please feel free to make any needed changes to the patch files in question in order to conform to the coding standards that OpenSSL 1.0.1c uses (I think I have kept to that format). I am attaching all three patch files to this email. A './config' and 'make' result in a clean compile of the patch files above, btw. Bill Parker (wp02855 at gmail dot com)
eng_cryptodev.c.patch
Description: Binary data
speed.c.patch
Description: Binary data
example.c.patch
Description: Binary data
cryptlib.c.patch
Description: Binary data
bn_asm.c.patch
Description: Binary data
bn_exp.c.patch
Description: Binary data
e_padlock.c.patch
Description: Binary data