Re: (ITS#8240) OpenLDAP ber_get_next denial of service vulnerability

[hyc@symas.com]

Michael Ströder wrote:
> hyc@symas.com wrote:
>> h.b.furuseth@usit.uio.no wrote:
>>> On 12/09/15 16:24, michael@stroeder.com wrote:
>>>> I've compiled with CFLAGS="-DNDEBUG" (also tried CPPFLAGS) but this did not
>>>> help. slapd still crashes when hitting the assert.
>>>
>>> Yes, portable.h #undefs it by default.  OpenLDAP has always conflated
>>> logging, debug output and asserts behind LDAP_DEBUG.  We've been saying
>>> for some time that we really ought to do something about that someday...
>>
>> Yes, and that's more obviously a bug that we can fix.
>
> Is it an easy fix?

Not for 2.4.

> I think that in opposite to the OpenLDAP project most people out there
> consider this to be a really serious bug to be fixed really soon.

> For now with my own builds I apply the patch removing the assert statement.
> But I wonder how many asserts statements are in the code which can be hit by
> invalid input leading to a crash.

Given the lack of OpenLDAP documentation about NDEBUG I have re-reverted my 
previous revert. The patch has been reinstated.

>> Every use of assert is "assert(the code is correct)" - but that often depends
>> on dynamic state, not just the statically written code.
>
> Yes, dynamic state including invalid input.  But IMO "assert(the code is
> correct)" should never be hit no matter how bad the input was.  And it should
> definitely not crash the server (with system's ressource limits being a
> unavoidable exception).  Rephrasing: The meaning of the statement "the code is
> correct" should also include "invalid input is properly handled as error" - no
> matter what.

In this particular case the code already handles the error perfectly well 
without the assert, so the assert doesn't serve any useful purpose.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/