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
