Re: Buffer overflows

On Friday 31 August 2007, Junio C Hamano wrote:

It is well and widely understood idiom to use strlcpy to a
fixed-sized buffer and checking the resulting length to make
sure the result would not have overflowed (and if it would have,
issue an error and die).  I would not have anything against a
set of patches to follow such a pattern.

But a patch to add a non-standard API that nobody else uses,
without any patch to show the changes to a few places that could
use the API to demonstrate that the use of API vastly cleans the
code up and makes it infinitely harder to make mistakes?

The API needs to justify itself to convince the people who needs
to learn and adjust to that the benefit far outweighes deviation
from better known patterns, and I do not see that happening in
Timo's patch.

So in general, git people seem to be saying that:

1. Yes, we agree that the C string library suX0rs badly.

2. There are more than 0 string manipulation bugs (e.g. buffer overflows) in 
git. The number may be small or large, but I have yet to see anyone claim 
it's _zero_.

3. Timo's patches (in their current form) are not the way to go, because of 
non-standard API, implementation problems, whatever...

So why does the discussion end there? Lukas proposed an interesting 
alternative in "The Better String Library" ( 
http://bstring.sourceforge.net/ ). Why has there been lots of bashing on 
Timo's efforts, but no critique of bstring? I'd be very keen to know what 
the git developers think of it. AFAICS, it seems to fulfill at least _some_ 
of the problems people find in Timo's patches. Specifically, it claims:

- High performance (better than the C string library)
- Simple usage

I'd also say it's probably more widely used than Timo's patches.


If the only response to Timo's highlighting of string manipulation problems 
in git, is for us to flame his patches and leave it at that, then I have no 
choice but to agree with him in that security does not seem to matter to 
us.


...Johan

-- 
Johan Herland, [off-list ref]
www.herland.net