Re: vulnerabilities in this code chunk

> ----- Message from [email protected] on 21 Jun 2007 22:41:04 -0000 -----
> I am trying to find all the vuln's in this code chunk, and the only 
> thing I can come up with is a null pointer dereference.  Assume data
> and data_len are user controlled.
> Null pointer happens when passing in a negative number.

You can't pass a negative number in an unsigned int - all numbers are 
non-negative.
And what do you mean by 'null pointer happens'?

> I was 
> looking hard at the memset functions but I couldn't come up with 
anything. 
> Anyone else see anything here?
> 
> char *copy_data(char *data, unsigned int data_len)
> { 
> unsigned int header_size = 8;
> char *buf;
> if (!(buf = malloc(data_len + header_size))) 
> {
> return NULL; 
> }
> memcpy(buf, "HEADER: ", 8);

Why not use header_size consistently?

> memcpy(buf + 8, data, data_len);
> return buf;
> }

Assuming 32-bit integers and data_len = 0xFFFFFFFC, this code requests 4 
bytes of data from malloc, and then tramples over 4GB of data.  Something 
is going to crash.

Similarly, if data_len is 0xFFFFFFF8, the code generates a request for 0 
bytes of data.  Some versions of malloc() -- I'm told that Windows is one 
such -- will return a valid pointer (rather than a null pointer) to zero 
bytes of usable space.  Crash again.  Numbers just a bit smaller than 
0xFFFFFFF8 are more likely to request too much memory and the malloc() 
should return a null pointer.

-- 
Jonathan Leffler ([email protected])
STSM, Informix Database Engineering, IBM Information Management Division
4100 Bohannon Drive, Menlo Park, CA 94025-1013
Tel: +1 650-926-6921    Tie-Line: 630-6921
"I don't suffer from insanity; I enjoy every minute of it!"