gnucash master: Fix wrong PWARN calling signature.

John Ralls jralls at ceridwen.us
Thu Jul 14 11:17:09 EDT 2016


> On Jul 14, 2016, at 7:04 AM, Derek Atkins <warlord at MIT.EDU> wrote:
> 
> John Ralls <jralls at ceridwen.us> writes:
> 
>>>> - PWARN(str.str().c_str());
>>>> + PWARN("%s", str.str().c_str());
>>>> What's the difference here?
>>>> 
>>>> 
>>> 
>>> I mean, I see that if the string is, for instance, "%s", nothing is output.
>>> But did this come up, or did you catch it visually while looking through
>>> the code.
>> 
>> master's nightly failed. I thought I'd tested it before committing,
>> but when I tried again this morning it failed.
>> 
>> The issue is that gcc will happily a single-argument invocation of
>> PWARN(format, ...) but g++ --at least the version in MinGW--won't.
> 
> Even worse, the original code could cause the app to crash if the
> provided string has some printf codes in it; causing printf to read into
> invalid memory.
> 
> This is the correct fix both programatically as well as from a security
> point of view.  You should never pass a (user-supplied) variable into
> the "format" input of a *printf function.
> 
> Thanks for fixing this, John.

Well, I made the error in the previous commit, so let's not get too effusive.

I'll change it again: In retrospect I see that it's dumb to pre-compose the string with stringstream only to pass it to printf.

There's no security threat here. The regtzi values are int32 from the Registry. Even if one were to edit those keys GnuCash would still read the bits as int32 and produce a string representation of the the base-10 numbers. '"%s"' would turn into 572879650.

Regards,
John Ralls




More information about the gnucash-devel mailing list