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 582a583 > ber_len_t extend_len; 583a585,596 > /* 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 ); > } >
Alan, Can you provide a "unified" diff. Ie: diff -u. Thanks. Kurt 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 >582a583 >> ber_len_t extend_len; >583a585,596 >> /* 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 ); >> } >> > > > >
diff -u -r1.1 encode.c --- encode.c 2000/03/02 19:51:37 1.1 +++ encode.c 2000/03/15 17:03:23 @@ -580,7 +580,20 @@ ber_len_t i; unsigned char nettag[sizeof(ber_tag_t)]; ber_tag_t tmptag = (*sos)->sos_tag; + 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 ); + } + /* the tag */ taglen = ber_calc_taglen( tmptag ); >>> "Kurt D. Zeilenga" <Kurt@OpenLDAP.org> 03/15/00 09:30AM >>> Alan, Can you provide a "unified" diff. Ie: diff -u. Thanks. Kurt 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 >582a583 >> ber_len_t extend_len; >583a585,596 >> /* 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 ); >> } >> > > > >
Changes committed. Please test. Thanks. Kurt
changed notes changed state Open to Test moved from Incoming to Software Bugs
changed state Test to Release
We have checked the fix. It looks great. Thanks. >>> "Kurt D. Zeilenga" <Kurt@OpenLDAP.org> 03/15/00 11:49AM >>> Changes committed. Please test. Thanks. Kurt
changed state Release to Closed
Applied to -devel and re1.2