[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Tim Ruehsen
Subject: Re: [Bug-wget] [PATCH] fix warning and possible invalid free
Date: Fri, 13 Apr 2012 10:44:57 +0200
User-agent: KMail/1.13.7 (Linux/3.2.0-2-amd64; KDE/4.7.4; x86_64; ; )

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 crashes though gcc did not complain !

To make gcc check for these potential crashes, use -Wwrite-strings.
From the gcc manual:
  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

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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]