[PATCH] Re: Possible Vulnerability in httpd.c

[Denys Vlasenko]

On Mon, Nov 21, 2016 at 8:18 PM, Simon Rettberg
<simon.rettberg at rz.uni-freiburg.de> wrote:
> On Mon, 21 Nov 2016 20:37:14 +0200
>  Timo Teras <timo.teras at iki.fi> wrote:
>>
>> It is still good practice to fill it with snprintf. If this is done,
>> proper error checking should be done to check the final 'len' that it
>> does not exceed IOBUF_SIZE or you have information leak bug (since
>> snprintf returns the length it would generate if buffer was unbounded;
>> not the length it actually wrote to the buffer).
>>
> Exactly. It typically goes like this: Someone is using functions generally
> considered "unsafe" because they know for sure it is not exploitable the way
> it's being used in this specific instance. Then eventually someone else comes
> along, adds feature X using unsafe functions as well (you try to do it the same
> way the rest is written, right?), and boom, you suddenly got your exploit
> because X happens to enable the remote user to inject arbitrarily long data
> (think some %s the user can control).

Yes, I am aware this happens.

> It should be fixed properly, handling the case where the return value is either
>> BUFSIZ or even < 0.

If you are afraid that there can be bugs of the "we thought it can't overflow,
but it in fact can", then why you aren't afraid that length checking code
wouldn't have bugs?

Different projects choose their paranoias differently.
>From its inception, bbox was paranoid about code size.

If you see an actual bug where buffer can overflow,
I'm more than willing to fix it.

But if there is no actual bug, and it's just a general concern
that "it looks unsafe", then code size trumps it.