[Date Prev][Date Next] [Chronological] [Thread] [Top]

Re: Heap corruption bug in encode.c (ITS#479)


Can you provide a "unified" diff.  Ie: diff -u.  Thanks.


At 03:05 PM 3/15/00 GMT, ACLARK@novell.com wrote:
>To:    openldap-its@OpenLDAP.org
>Kudos to Dave Steck for the find and fix
>There is a situation where ber_put_seqorset() writes a few bytes beyond an allocated buffer, causing a nasty heap corruption bug that makes you crash at some random later time.
>The problem occurs when:
>	a) You're encoding something like [V] .
>	b) You specify NULL for V.
>  and	c) The internal buffer of the BerElement is within a few bytes of the end
>This type of encoding is used when removing all values of an attribute type, for example.
>The '[' code causes the pointer into the buffer to be incremented a few bytes, but it doesn't write anything.  It can actually be incremented beyond the end of the buffer and that's OK.  When you actually do the ber_write, it reallocs the buffer if necessary.
>However, if the V is NULL, no ber_write occurs.  When it sees the ']' code, it updates the tag and length fields, which may now be off the end of the buffer.
>The solution is to check for this condition in ber_put_seqorset(), and realloc the buffer if necessary before writing into those fields.  We also had to make ber_realloc non-static and move the prototype into lber-int.h.
>diff -r1.1 encode.c
>> 		ber_len_t extend_len;
>> 		/* Check if sos_ptr has exceeded buffer.  This can happen with [V]
>> 		   for example if the ptr was close to the end of the buffer
>> 		   and no data was written for the 'V'.  If so, we must do a realloc
>> 		   before doing the SAFEMEMCPY's, which write into the tag and
>> 		   length fields.
>> 		*/
>> 		if ( ber->ber_sos->sos_ptr > ber->ber_end ) {
>> 			extend_len = ber->ber_sos->sos_ptr - ber->ber_end;
>> 			if ( ber_realloc( ber, extend_len ) != 0 )
>> 				return( -1 );
>> 		}