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

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

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 );
> 		}