Commit d9a9d10f authored by Dr. Stephen Henson's avatar Dr. Stephen Henson
Browse files

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)
parent 0d2baadf

CHANGES

+10 −1
Original line number Diff line number Diff line
@@ -289,7 +289,16 @@ 
     whose return value is often ignored. 

     [Steve Henson]

  

 Changes between 1.0.1 and 1.0.1a [xx XXX xxxx]

 Changes between 1.0.1 and 1.0.1a [19 Apr 2012]



  *) 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]



  *) Don't allow TLS 1.2 SHA-256 ciphersuites in TLS 1.0, 1.1 connections.

     [Adam Langley]

crypto/asn1/a_d2i_fp.c

+40 −14
Original line number Diff line number Diff line
@@ -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,8 +176,15 @@ 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 */



		p=(unsigned char *)&(b->data[off]);
@@ -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;

					}

				}

			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;

	}

crypto/buffer/buffer.c

+17 −0
Original line number Diff line number Diff line
@@ -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, size_t 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, size_t 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);

crypto/mem.c

+4 −0
Original line number Diff line number Diff line
@@ -364,6 +364,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);