[GRASS5] str*() vs strn*() functions

[Paul Kelly]

On Fri, 26 Aug 2005, Brad Douglas wrote:

> On Fri, 2005-08-26 at 10:18 +0100, Paul Kelly wrote:
>> On Fri, 26 Aug 2005, Brad Douglas wrote:
>>
>>> Is there any particular reason there has been a recent move from using
>>> strn*() function to using str*() functions?
>>
>> It was a bugfix: with strncmp it was returning a match if one string was
>> shorter than the other but matched the first n characters. We needed to
>> match the whole string.
>
> Is this a comparison of two strings of arbitrary length or is the source
> string known?

Well yes---we have a specific pre-determined keyword that we are trying to 
match against one read from a while. I presumed that after strcmp got to 
the end of the shorter string it would stop matching, which removes the 
possibility of a buffer overflow. I haven't found any information on this 
possiblity, and the man page for strcmp doesn't mention it.

> Do you have an example of where it failed to work
> properly?  I'm curious.

It was discussed on the list two days ago:
http://grass.itc.it/pipermail/grass5/2005-August/019378.html

>>> Specifying the string length has security benefits.
>>
>> What is the problem with using strcmp specifically?
>
> Buffer overflow attacks.
>
>> I had a think about it and couldn't think of any reason not to use strcmp(),
>> so I changed some occurences of strncmp() I had added in the past (blindly
>> following the way it was done in other parts of the proj library).
>
> I found a short article detailing the problem for anyone interested:
> http://www.linuxsecurity.com/content/view/119087/49/

The article seems to deal with comparing two user-supplied strings. So I 
don't think it's relevant. Also there is only a passing reference to strcmp /
strncmp and it doesn't explain what the issue with it is. As far as I 
understand it comparing two strings can't cause a buffer overflow in 
itself; the damage has to be done at the stage where the strings are read 
into memory?

There's been a lot of work done in GRASS in the
last few years to reduce the chances of buffer overflows (G_asprintf() etc.) 
although many unsafe assignments/comparisons still remain. I'd like to 
think I'm not adding any new ones though :)

Paul