Check for potentially exploitable overflows in asn1_d2i_read_bio

BUF_mem_grow and BUF_mem_grow_clean. Refuse attempts to shrink buffer
in CRYPTO_realloc_clean.

Thanks to Tavis Ormandy, Google Security Team, for discovering this
issue and to Adam Langley <agl@chromium.org> for fixing it. (CVE-2012-2110)

diff --git a/CHANGES b/CHANGES
index b3424e898e267dc6eb092f623afd0234e32c8abf..a2275713fb3d1c6b074432a9994edd9e6b70ff1c 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -4,7 +4,14 @@
 
  Changes between 0.9.8u and 0.9.8v [xx XXX xxxx]
 
-  *)
+  *) Check for potentially exploitable overflows in asn1_d2i_read_bio
+     BUF_mem_grow and BUF_mem_grow_clean. Refuse attempts to shrink buffer
+     in CRYPTO_realloc_clean.
+
+     Thanks to Tavis Ormandy, Google Security Team, for discovering this
+     issue and to Adam Langley <agl@chromium.org> for fixing it.
+     (CVE-2012-2110)
+     [Adam Langley (Google), Tavis Ormandy, Google Security Team]
 
  Changes between 0.9.8t and 0.9.8u [12 Mar 2012]
 

diff --git a/crypto/asn1/a_d2i_fp.c b/crypto/asn1/a_d2i_fp.c
index ece40bc4c0033ba075178c8548d2c56ccf2819eb..52b2ebdb631b9941123eb605cdc6564ed32c8739 100644 (file)
--- a/crypto/asn1/a_d2i_fp.c
+++ b/crypto/asn1/a_d2i_fp.c
@@ -57,6 +57,7 @@
  */
 
 #include <stdio.h>
+#include <limits.h>
 #include "cryptlib.h"
 #include <openssl/buffer.h>
 #include <openssl/asn1_mac.h>
@@ -143,17 +144,11 @@ static int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
        BUF_MEM *b;
        unsigned char *p;
        int i;
-       int ret=-1;
        ASN1_const_CTX c;
-       int want=HEADER_SIZE;
+       size_t want=HEADER_SIZE;
        int eos=0;
-#if defined(__GNUC__) && defined(__ia64)
-       /* pathetic compiler bug in all known versions as of Nov. 2002 */
-       long off=0;
-#else
-       int off=0;
-#endif
-       int len=0;
+       size_t off=0;
+       size_t len=0;
 
        b=BUF_MEM_new();
        if (b == NULL)
@@ -169,7 +164,7 @@ static int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
                        {
                        want-=(len-off);
 
-                       if (!BUF_MEM_grow_clean(b,len+want))
+                       if (len + want < len || !BUF_MEM_grow_clean(b,len+want))
                                {
                                ASN1err(ASN1_F_ASN1_D2I_READ_BIO,ERR_R_MALLOC_FAILURE);
                                goto err;
@@ -181,7 +176,14 @@ static int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
                                goto err;
                                }
                        if (i > 0)
+                               {
+                               if (len+i < len)
+                                       {
+                                       ASN1err(ASN1_F_ASN1_D2I_READ_BIO,ASN1_R_TOO_LONG);
+                                       goto err;
+                                       }
                                len+=i;
+                               }
                        }
                /* else data already loaded */
 
@@ -206,6 +208,11 @@ static int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
                        {
                        /* no data body so go round again */
                        eos++;
+                       if (eos < 0)
+                               {
+                               ASN1err(ASN1_F_ASN1_D2I_READ_BIO,ASN1_R_HEADER_TOO_LONG);
+                               goto err;
+                               }
                        want=HEADER_SIZE;
                        }
                else if (eos && (c.slen == 0) && (c.tag == V_ASN1_EOC))
@@ -220,10 +227,16 @@ static int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
                else 
                        {
                        /* suck in c.slen bytes of data */
-                       want=(int)c.slen;
+                       want=c.slen;
                        if (want > (len-off))
                                {
                                want-=(len-off);
+                               if (want > INT_MAX /* BIO_read takes an int length */ ||
+                                       len+want < len)
+                                               {
+                                               ASN1err(ASN1_F_ASN1_D2I_READ_BIO,ASN1_R_TOO_LONG);
+                                               goto err;
+                                               }
                                if (!BUF_MEM_grow_clean(b,len+want))
                                        {
                                        ASN1err(ASN1_F_ASN1_D2I_READ_BIO,ERR_R_MALLOC_FAILURE);
@@ -238,11 +251,18 @@ static int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
                                                    ASN1_R_NOT_ENOUGH_DATA);
                                                goto err;
                                                }
+                                       /* This can't overflow because
+                                        * |len+want| didn't overflow. */
                                        len+=i;
-                                       want -= i;
+                                       want-=i;
                                        }
                                }
-                       off+=(int)c.slen;
+                       if (off + c.slen < off)
+                               {
+                               ASN1err(ASN1_F_ASN1_D2I_READ_BIO,ASN1_R_TOO_LONG);
+                               goto err;
+                               }
+                       off+=c.slen;
                        if (eos <= 0)
                                {
                                break;
@@ -252,9 +272,15 @@ static int asn1_d2i_read_bio(BIO *in, BUF_MEM **pb)
                        }
                }
 
+       if (off > INT_MAX)
+               {
+               ASN1err(ASN1_F_ASN1_D2I_READ_BIO,ASN1_R_TOO_LONG);
+               goto err;
+               }
+
        *pb = b;
        return off;
 err:
        if (b != NULL) BUF_MEM_free(b);
-       return(ret);
+       return -1;
        }

diff --git a/crypto/buffer/buffer.c b/crypto/buffer/buffer.c
index b3e947771d5850246f571457bdce1f4d328650c0..3fa7229977965155fa9d1690c471cdf678b37dd8 100644 (file)
--- a/crypto/buffer/buffer.c
+++ b/crypto/buffer/buffer.c
@@ -60,6 +60,11 @@
 #include "cryptlib.h"
 #include <openssl/buffer.h>
 
+/* LIMIT_BEFORE_EXPANSION is the maximum n such that (n+3)/3*4 < 2**31. That
+ * function is applied in several functions in this file and this limit ensures
+ * that the result fits in an int. */
+#define LIMIT_BEFORE_EXPANSION 0x5ffffffc
+
 BUF_MEM *BUF_MEM_new(void)
        {
        BUF_MEM *ret;
@@ -105,6 +110,12 @@ int BUF_MEM_grow(BUF_MEM *str, int len)
                str->length=len;
                return(len);
                }
+       /* This limit is sufficient to ensure (len+3)/3*4 < 2**31 */
+       if (len > LIMIT_BEFORE_EXPANSION)
+               {
+               BUFerr(BUF_F_BUF_MEM_GROW,ERR_R_MALLOC_FAILURE);
+               return 0;
+               }
        n=(len+3)/3*4;
        if (str->data == NULL)
                ret=OPENSSL_malloc(n);
@@ -142,6 +153,12 @@ int BUF_MEM_grow_clean(BUF_MEM *str, int len)
                str->length=len;
                return(len);
                }
+       /* This limit is sufficient to ensure (len+3)/3*4 < 2**31 */
+       if (len > LIMIT_BEFORE_EXPANSION)
+               {
+               BUFerr(BUF_F_BUF_MEM_GROW,ERR_R_MALLOC_FAILURE);
+               return 0;
+               }
        n=(len+3)/3*4;
        if (str->data == NULL)
                ret=OPENSSL_malloc(n);

diff --git a/crypto/mem.c b/crypto/mem.c
index 00ebaf0b9b5e304b860b4443c07dc2a45837da6b..05d7b9cd3d7aa7d17f859050aa7fe14d5e3cdcbb 100644 (file)
--- a/crypto/mem.c
+++ b/crypto/mem.c
@@ -372,6 +372,10 @@ void *CRYPTO_realloc_clean(void *str, int old_len, int num, const char *file,
 
        if (num <= 0) return NULL;
 
+       /* We don't support shrinking the buffer. Note the memcpy that copies
+        * |old_len| bytes to the new buffer, below. */
+       if (num < old_len) return NULL;
+
        if (realloc_debug_func != NULL)
                realloc_debug_func(str, NULL, num, file, line, 0);
        ret=malloc_ex_func(num,file,line);