Re: [Speed-dreams-devel] Speed dreams stack based buffer overflow

[kilo a. G. K. <kg...@gm...>]

Hi Andres,

I've implemented the changes you'd suggested, see rev 4487:
http://sourceforge.net/apps/trac/speed-dreams/changeset/4487

Thanks for the notice and please don't hesitate to share with us if
you have any other suggestions.

cheers
kilo
--
http://three.sentenc.es



On 8 February 2012 17:10, Andres Gomez <ag...@fl...> wrote:
> Hi Kilo,
>
> I didn't know snprintf was so problematic,  so well I think a line like:
>
>
> sprintf(buf, "%.*s", sizeof(buf), something_very_long_causing_overflow);
>
> could get the work done, and also is more portable, so I agree, It is a
> better solution.
>
> Regards.
>
> 2012/2/7 kilo aka Gabor Kmetyko <kg...@gm...>
>>
>> Hi Andres,
>>
>> On 4 February 2012 18:23, Andres Gomez <and...@fl...>
>> wrote:
>> > Hi Kilo,
>> >
>> > No, it is not the same, it is another exploitable buffer overflow in
>> > torcs
>> > and speed dreams(2 and previous versions), this time it does'nt have
>> > relation with plib.
>> >
>> > The problem is in:
>> >
>> > speed-dreams/src/modules/graphic/ssgraph/grsound.cpp, line 93:
>> >
>> > if audio file name in "engine sample" is enough long it could overwrite
>> > "filename" buffer (line 86),
>> > because there is not size validation in line 93 (also in line 97).
>> > The Solution would be to use snprintf taking care of buffer's size
>> > (512).
>>
>>
>> I see your point.
>>
>> However when I hear "snprintf" a small bell rings in the back of my
>> head calling out "something nasty there":
>> IIRC there are/were issues with snprintf() itself, I don't even know
>> which C++ standard it is in. And I am pretty sure there were unsafe
>> versions of it, not checking the limits at all. Which OS, which lib, I
>> really don't remember...
>>
>> So maybe better to build a custom override "wrapper" around sprintf()
>> that checks for boundaries _for_sure_? Or does the precision flag
>> solve the problem:
>>
>>  char buf[20];
>>  sprintf(buf, "%.20s", something_very_long_causing_overflow);
>>
>> or even better:
>>  sprintf(buf, "%.*s", sizeof(buf), something_very_long_causing_overflow);
>>
>> Please share what you think about this issue.
>>
>> cheers
>> kilo
>> --
>> http://three.sentenc.es
>>
>>
>> ------------------------------------------------------------------------------
>> Keep Your Developer Skills Current with LearnDevNow!
>>
>> The most comprehensive online learning library for Microsoft developers
>> is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3, MVC3,
>> Metro Style Apps, more. Free future releases when you subscribe now!
>> http://p.sf.net/sfu/learndevnow-d2d
>>
>> _______________________________________________
>> Speed-dreams-devel mailing list
>> Spe...@li...
>> https://lists.sourceforge.net/lists/listinfo/speed-dreams-devel
>
>