[openssl-dev] [openssl.org #4374] [PATCH] Potential for NULL pointer dereferences in OpenSSL-1.0.2g (CWE-476)

Bill Parker via RT rt at openssl.org
Thu Mar 3 20:46:37 UTC 2016


Hello All,

In reviewing source code in directory 'openssl-1.0.2g/crypto/evp',
in file 'openbsd_hw.c', there are a few instances where OPENSSL_malloc()
is called, but immediately afterwards a call to memcpy() is made with
the return value from the call to OPENSSL_malloc(), but no check for
a return value of NULL is made after OPENSSL_malloc() returns.

However, if the 1st argument to memcpy() is NULL, a segmentation fault/
violation will occur.  The patch file below should address/correct this
issue:

--- openbsd_hw.c.orig   2016-03-02 15:36:57.236927351 -0800
+++ openbsd_hw.c        2016-03-02 15:40:29.525908189 -0800
@@ -133,6 +133,10 @@
         return 0;

     CDATA(ctx)->key = OPENSSL_malloc(MAX_HW_KEY);
+    if (CDATA(ctx)->key == NULL {
+       err("CDATA(ctx)->key memory allocation failed");
+       return 0;
+    }

     assert(ctx->cipher->iv_len <= MAX_HW_IV);

@@ -186,6 +190,11 @@

             if (((unsigned long)in & 3) || cinl != inl) {
                 cin = OPENSSL_malloc(cinl);
+               if (cin == NULL) {
+                   err("cin - memory allocation failed");
+                   abort();
+                   return 0;
+               }
                 memcpy(cin, in, inl);
                 cryp.src = cin;
             }
@@ -334,6 +343,11 @@
             char *dcopy;

             dcopy = OPENSSL_malloc(len);
+           if (dcopy == NULL) {
+               err("dcopy - memory allocation failed");
+               abort();
+               return 0;
+           }
             memcpy(dcopy, data, len);
             cryp.src = dcopy;
             cryp.dst = cryp.src; // FIXME!!!
@@ -397,6 +411,10 @@
     assert(from->digest->flags & EVP_MD_FLAG_ONESHOT);

     to_md->data = OPENSSL_malloc(from_md->len);
+    if (to_md->data == NULL) {
+       err("DEV_CRYPTO_MD5_COPY: unable to allocate memory");
+       return 0;
+    }
     memcpy(to_md->data, from_md->data, from_md->len);

     return 1;

=======================================================================

Hello All,

In reviewing source code in directory 'engines/ccgost', in file
'gost_ameth.c', there are a few instances where OPENSSL_malloc()
is called, but no check for a return value of NULL is made.  However,
immediately afterwards statments which access the allocated memory
are used (array access/memset(), etc) which will result in a segmentation
fault/violation occuring if NULL is returned from the OPENSSL_malloc()
call.

The patch file below should address/correct this issue:

--- gost_ameth.c.orig   2016-03-02 16:43:36.014151374 -0800
+++ gost_ameth.c        2016-03-02 16:45:59.978448496 -0800
@@ -617,6 +617,10 @@
         return 0;
     }
     databuf = OPENSSL_malloc(octet->length);
+    if (!databuf) {
+       GOSTerr(GOST_F_PUB_DECODE_GOST94, ERR_R_MALLOC_FAILURE);
+       return 0;
+    }
     for (i = 0, j = octet->length - 1; i < octet->length; i++, j--) {
         databuf[j] = octet->data[i];
     }
@@ -646,6 +650,8 @@
     }
     data_len = BN_num_bytes(dsa->pub_key);
     databuf = OPENSSL_malloc(data_len);
+    if (!databuf)
+       return 0;
     BN_bn2bin(dsa->pub_key, databuf);
     octet = ASN1_OCTET_STRING_new();
     ASN1_STRING_set(octet, NULL, data_len);
@@ -686,6 +692,10 @@
         return 0;
     }
     databuf = OPENSSL_malloc(octet->length);
+    if (!databuf) {
+       GOSTerr(GOST_F_PUB_DECODE_GOST01, ERR_R_MALLOC_FAILURE);
+       return 0;
+    }
     for (i = 0, j = octet->length - 1; i < octet->length; i++, j--) {
         databuf[j] = octet->data[i];
     }
@@ -760,6 +770,10 @@
     data_len = 2 * BN_num_bytes(order);
     BN_free(order);
     databuf = OPENSSL_malloc(data_len);
+    if (!databuf) {
+       GOSTerr(GOST_F_PUB_DECODE_GOST01, ERR_R_MALLOC_FAILURE);
+       return 0;
+    }
     memset(databuf, 0, data_len);

     store_bignum(X, databuf + data_len / 2, data_len / 2);

=======================================================================

Hello All,

In reviewing source code in directory 'engines/ccgost', in file
'gost_pmeth.c', there are a few instances where OPENSSL_malloc()
is called, but no check for a return value of NULL is made.  However,
immediately afterwards statments which access the allocated memory
are used (memcpy()/memset(), etc) which will result in a segmentation
fault/violation occuring if NULL is returned from the OPENSSL_malloc()
call.

The patch file below should address/correct this issue:

--- gost_pmeth.c.orig   2016-03-02 17:24:49.503519153 -0800
+++ gost_pmeth.c        2016-03-02 17:27:27.179558967 -0800
@@ -107,6 +107,8 @@
         return 1;
     case EVP_PKEY_CTRL_SET_IV:
         pctx->shared_ukm = OPENSSL_malloc((int)p1);
+       if (!pctx->shared_ukm)
+           return 0;
         memcpy(pctx->shared_ukm, p2, (int)p1);
         return 1;
     case EVP_PKEY_CTRL_PEER_KEY:
@@ -533,6 +535,8 @@
         return 0;
     }
     keydata = OPENSSL_malloc(32);
+    if (!keydata)
+       return 0;
     memcpy(keydata, data->key, 32);
     EVP_PKEY_assign(pkey, NID_id_Gost28147_89_MAC, keydata);
     return 1;

=======================================================================

Hello All,

In reviewing source code in directory 'ssl', in file 'd1_both.c',
there are a few instances where OPENSSL_malloc() is called, but no
check for a return value of NULL is made.  However, immediately
afterwards statments which access the allocated memory
are used (memcpy()/memset(), etc) which will result in a segmentation
fault/violation occuring if NULL is returned from the OPENSSL_malloc()
call.

The patch file below should address/correct this issue:

--- d1_both.c.orig      2016-03-02 17:31:30.838526769 -0800
+++ d1_both.c   2016-03-02 17:33:49.002086647 -0800
@@ -1459,6 +1459,8 @@
          * plus 2 bytes payload length, plus payload, plus padding
          */
         buffer = OPENSSL_malloc(write_length);
+       if (buffer == NULL)
+           return -1; /* what should be returned here???   */
         bp = buffer;

         /* Enter response type, length and copy payload */
@@ -1544,6 +1546,8 @@
      *  - Padding
      */
     buf = OPENSSL_malloc(1 + 2 + payload + padding);
+    if (!buf)
+       goto err;
     p = buf;
     /* Message Type */
     *p++ = TLS1_HB_REQUEST;

=======================================================================

Hello All,

In reviewing source code in directory 'ssl', in file 'd1_both.c',
there is a instance where OPENSSL_malloc() is called, but no
check for a return value of NULL is made.  However, immediately
afterwards a call to memcpy() is made, but if the return value
from OPENSSL_malloc() is NULL, a segmentation fault/violation will
occur.

The patch file below should address/correct this issue:

--- s3_clnt.c.orig      2016-03-02 17:43:33.256342358 -0800
+++ s3_clnt.c   2016-03-02 17:44:48.744936571 -0800
@@ -2111,6 +2111,10 @@
     if (ctype_num > SSL3_CT_NUMBER) {
         /* If we exceed static buffer copy all to cert structure */
         s->cert->ctypes = OPENSSL_malloc(ctype_num);
+       if (s->cert->ctypes == NULL) {
+           SSLerr(SSL_F_SSL3_GET_NEW_SESSION_TICKET, ERR_R_MALLOC_FAILURE);
+           goto err;
+       }
         memcpy(s->cert->ctypes, p, ctype_num);
         s->cert->ctype_num = (size_t)ctype_num;
         ctype_num = SSL3_CT_NUMBER;

=======================================================================

Hello All,

In reviewing source code in directory 'ssl', in file 'ssl_sess.c',
there is a instance where OPENSSL_malloc() is called, but no
check for a return value of NULL is made.  However, immediately
afterwards a call to memcpy() is made, but if the return value
from OPENSSL_malloc() is NULL, a segmentation fault/violation will
occur.

The patch file below should address/correct this issue:

--- ssl_sess.c.orig     2016-03-02 17:48:47.180240472 -0800
+++ ssl_sess.c  2016-03-02 17:50:20.204063321 -0800
@@ -919,6 +919,10 @@
             session->krb5_client_princ_len > 0) {
             s->kssl_ctx->client_princ =
                 (char *)OPENSSL_malloc(session->krb5_client_princ_len + 1);
+           if (!s->kssl_ctx->client_princ) {
+               SSLerr(SSL_F_SSL_SESSION_NEW, ERR_R_MALLOC_FAILURE);
+               return (0);
+           }
             memcpy(s->kssl_ctx->client_princ, session->krb5_client_princ,
                    session->krb5_client_princ_len);
             s->kssl_ctx->client_princ[session->krb5_client_princ_len] =
'\0';

=======================================================================

Hello All,

In reviewing source code in directory 'ssl', in file 's3_enc.c',
there is a instance where OPENSSL_malloc() is called, but no
check for a return value of NULL is made.  However, immediately
afterwards a call to memset() is made, but if the return value
from OPENSSL_malloc() is NULL, a segmentation fault/violation will
occur.

The patch file below should address/correct this issue:

--- s3_enc.c.orig       2016-03-02 17:53:14.248183434 -0800
+++ s3_enc.c    2016-03-02 17:55:05.883371692 -0800
@@ -607,6 +607,10 @@
     ssl3_free_digest_list(s);
     s->s3->handshake_dgst =
         OPENSSL_malloc(SSL_MAX_DIGEST * sizeof(EVP_MD_CTX *));
+    if (s->s3->handshake_dgst == NULL) {
+       SSLerr(SSL_F_SSL3_CHANGE_CIPHER_STATE, ERR_R_MALLOC_FAILURE);
+       return 0;
+    }
     memset(s->s3->handshake_dgst, 0, SSL_MAX_DIGEST * sizeof(EVP_MD_CTX
*));
     hdatalen = BIO_get_mem_data(s->s3->handshake_buffer, &hdata);
     if (hdatalen <= 0) {

========================================================================

Bill Parker (wp02855 at gmail dot com)

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4374
Please log in as guest with password guest if prompted

-------------- next part --------------
A non-text attachment was scrubbed...
Name: openbsd_hw.c.patch
Type: application/octet-stream
Size: 1299 bytes
Desc: not available
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20160303/97cacfc6/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gost_ameth.c.patch
Type: application/octet-stream
Size: 1312 bytes
Desc: not available
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20160303/97cacfc6/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gost_pmeth.c.patch
Type: application/octet-stream
Size: 603 bytes
Desc: not available
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20160303/97cacfc6/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: d1_both.c.patch
Type: application/octet-stream
Size: 618 bytes
Desc: not available
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20160303/97cacfc6/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: s3_clnt.c.patch
Type: application/octet-stream
Size: 548 bytes
Desc: not available
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20160303/97cacfc6/attachment-0004.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ssl_sess.c.patch
Type: application/octet-stream
Size: 623 bytes
Desc: not available
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20160303/97cacfc6/attachment-0005.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: s3_enc.c.patch
Type: application/octet-stream
Size: 544 bytes
Desc: not available
URL: <http://mta.openssl.org/pipermail/openssl-dev/attachments/20160303/97cacfc6/attachment-0006.obj>


More information about the openssl-dev mailing list