Re: [Modcplusplus-devel] (johnksterling) mod_cplusplus/src cpp_request.cpp

[Jonathan W. <co...@co...>]

Re-sending as I screwed it up ...

On Sat, Jul 31, 2004 at 06:58:07AM -0700, Mod Cplusplus CVS List wrote:

> -void ApacheRequestRec::dump()
> +// translate integer to ascii string
> +inline string
> +istring(int value, const char* format = "%d")
> +{
> +    enum { TEMP_STORE = 50 };
> +    char temp[TEMP_STORE];
> +    sprintf(temp,format,value);
> +    return string(temp);
> +}

Could a stringstream be used here instead?
It is Mod _Cplusplus_ after all.

That avoids using fixed-length buffers and format strings, two potential
security problems.

The function could even be made into a template to support more than
integers, but that might be overkill.

If using sprintf(3) is needed for portability or other reasons, how about
the safer snprintf(3) ?  Or removing the format string argument ?  Does
the (unused) flexibility of supplying a format string warrant the
potential problems caused by passing in a format string that results in
more than 50 chars (buffer overrun) or passing in a string that is
incompatible with the integer argument, e.g. "%s" ?

AFAICT that function isn't in an anonymous namespace, and has external
linkage, so could potentially be called from outside that file. IMHO
that means it should be safe against buffer overflows and format string
injection.

Maybe I'm being too paranoid - but that's the frame of mind I like to
adopt when writing apache module code. Web servers are exposed to every
nasty person on the net and I don't think it hurts to be paranoid.

Finally, the return value of sprintf(3) could be used to speed up
the string ctor:

    const int len = sprintf(temp,format,value);
    return string(temp, len);

regards,

jon

-- 
"The whole problem with the world is that fools and fanatics are always
 so certain of themselves, but wiser people so full of doubts."
	- Bertrand Russell