There are many places in our code where we assume that if ber_scanf returns an error, this means it did not allocate any memory. That is not the case. ber_get_stringa first reads the length, then allocates the memory to read into, then does the actual reading. If it receives an error from ber_read while reading the buffer, it will short circuit and return with an allocated (but uninitialized) buffer that must be freed by the caller. ber_get_stringal does not have this problem, but suffers from a different problem which will be addressed in the ldap sdk. There is a similar problem with using ber_scanf 'v' or 'V' to read in arrays. However, in that case, cleanup is problematic due to a bug in the ldap sdk - after allocating space for the array, we do not set the array element (the string) to NULL, so if we try to free it, we get a free of uninitialized memory. This will also be fixed in the ldap sdk.
Created attachment 123783 [details] files for fix
Created attachment 123784 [details] diffs for fix
Note of CVE names: CVE-2006-0451: A number of memory leaks due to BER error handling can cause the server to run out of memory and exit CVE-2006-0453: A bad BER sequence can be handled incorrectly and lead to a crash due to unitialized memory free.
Checking in ldapserver/ldap/servers/plugins/replication/repl5_total.c; /cvs/dirsec/ldapserver/ldap/servers/plugins/replication/repl5_total.c,v <-- repl5_total.c new revision: 1.6; previous revision: 1.5 done Checking in ldapserver/ldap/servers/plugins/replication/repl_controls.c; /cvs/dirsec/ldapserver/ldap/servers/plugins/replication/repl_controls.c,v <-- repl_controls.c new revision: 1.6; previous revision: 1.5 done Checking in ldapserver/ldap/servers/plugins/replication/repl_extop.c; /cvs/dirsec/ldapserver/ldap/servers/plugins/replication/repl_extop.c,v <-- repl_extop.c new revision: 1.8; previous revision: 1.7 done Checking in ldapserver/ldap/servers/slapd/add.c; /cvs/dirsec/ldapserver/ldap/servers/slapd/add.c,v <-- add.c new revision: 1.6; previous revision: 1.5 done Checking in ldapserver/ldap/servers/slapd/ava.c; /cvs/dirsec/ldapserver/ldap/servers/slapd/ava.c,v <-- ava.c new revision: 1.5; previous revision: 1.4 done Checking in ldapserver/ldap/servers/slapd/bind.c; /cvs/dirsec/ldapserver/ldap/servers/slapd/bind.c,v <-- bind.c new revision: 1.7; previous revision: 1.6 done Checking in ldapserver/ldap/servers/slapd/compare.c; /cvs/dirsec/ldapserver/ldap/servers/slapd/compare.c,v <-- compare.c new revision: 1.5; previous revision: 1.4 done Checking in ldapserver/ldap/servers/slapd/delete.c; /cvs/dirsec/ldapserver/ldap/servers/slapd/delete.c,v <-- delete.c new revision: 1.5; previous revision: 1.4 done Checking in ldapserver/ldap/servers/slapd/filter.c; /cvs/dirsec/ldapserver/ldap/servers/slapd/filter.c,v <-- filter.c new revision: 1.6; previous revision: 1.5 done Checking in ldapserver/ldap/servers/slapd/modify.c; /cvs/dirsec/ldapserver/ldap/servers/slapd/modify.c,v <-- modify.c new revision: 1.9; previous revision: 1.8 done Checking in ldapserver/ldap/servers/slapd/modrdn.c; /cvs/dirsec/ldapserver/ldap/servers/slapd/modrdn.c,v <-- modrdn.c new revision: 1.5; previous revision: 1.4 done Checking in ldapserver/ldap/servers/slapd/back-ldbm/sort.c; /cvs/dirsec/ldapserver/ldap/servers/slapd/back-ldbm/sort.c,v <-- sort.c new revision: 1.6; previous revision: 1.5 done
Reviewed by: All (Thanks!) Files: https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=123783 Branch: HEAD Fix Description: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=179135#c0 I basically did a search through our code for all calls to ber_scanf, ber_get_stringa, and ber_get_stringal and made sure we properly free any arguments that may have been allocated. There was a bug in the ldapsdk https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=179135 that causes us to free uninitialized memory when trying to clean up the result of ber_get_stringal (or ber_scanf with 'V'). I had to initialize some variables to NULL so that we could properly clean them up, and added some additional clean ups that were missing. Also, in repl_extop.c, we were calling free on an array that we should have been calling ch_array_free on. Yet another lesson in the evils of slapi_ch_free and disabling compiler type checks in general. Platforms tested: Fedora Core 4 Flag Day: no Doc impact: no
Ran various iterations of Rich's ber test script (Thanks a lot Rich!). Verified against DS 7.1 SP2 20060314.1 and DS 6.21 SP3 20060310.1. 20k loop runs in addition to 100k loop runs didn't show significant memory usage increase against the above candidate builds. Note that 100k runs took about 3 weeks to complete, while 20k took about 2 days.
Somehow the errata system did not automatically close these bugs even though DS SP 2 is shipped and available live on RHN
trying to manually close