Re: [Bug-wget] [PATCH] fix warning and possible invalid free

[Tim Ruehsen]

Am Thursday 12 April 2012 schrieb Micah Cowan:
> On 04/12/2012 01:23 AM, TeeRasen wrote:
> > In main.c we have
> > 
> >           opt.progress_type = "dot";
> 
> In C, a string literal is of type char[] (which automatically transforms
> to char*), not const char[] or const char* (even though one must still
> not modify it. You're either compiling with C++ (a bad idea for wget
> code), or a nonstandard C invocation that makes string literals out to
> be const. (Though, a C implementation is allowed to "warn" about
> whatever it wishes, provided it still behaves properly, I suppose.)

Maybe, my posting was too long to read...
The main message was that free("dot") crashes, since literals are read-only. 
They have been read-only since CPUs have MMUs.
Assigning the address of a literal to a char * does not change that and is a 
security risk, not only in my eyes. 

Example (compile with gcc x.c -o x):
#include <stdlib.h>
void main(void)
{
        char *buf="dot";
        free(buf);
}
free crashes though gcc did not complain !

To make gcc check for these potential crashes, use -Wwrite-strings.
From the gcc manual:
 -Wwrite-strings
  When compiling C, give string constants the type "const char[length]" so 
that copying the address of one into a non-"const" "char *" pointer will get a 
warning.  These warnings will help you find at compile time code that can try 
to write into a string constant, but only if you have been very careful about 
using "const" in declarations and prototypes.  Otherwise, it will just be a 
nuisance. This is why we did not make -Wall request these warnings.

With other words:
There is so many crappy source code out there, that hackers become overwhelmed 
by warnings and rather turn them off instead of fixing the code.

I found lot's of issues in wget just by turning on gcc's extra warnings. And i 
am far away from having all the issues patched. But i am working on it.


Wget's source code widely uses 'const' to prevent issues, which is a good 
thing. But on some places 'const' is missing. I would like to provide further 
patches for these places, as long there is support from you maintainers.

Good starting points for further reading about securing / hardening and 
compiler flags for developers are
http://wiki.debian.org/Hardening
http://stackoverflow.com/questions/154630/recommended-gcc-warning-options-for-
c

BTW, since warc.c is C99 code shouldn't we strive to lift the rest of the code 
(step by step) into C99 area as well ? I started to make a list of things 
which I found when fixing things... there is really much to do and if you 
don't mind, I would like to put some efford into that. But warnings and bugs 
first ;-)

> 
> -mjc

     Tim