This is the mail archive of the cygwin mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Using malloc/realloc along with gdb: heap overflows


See comments below...

On 8/8/07, Eric Belanger  wrote:

> ---- Code ----
> #include <stdlib.h>
> #include <string.h>
> #include <stdio.h>
>
> #define TCP_BUFSIZE 2
>
> int main(int argc, char *argv) {
>     /* *bufdata and *alldata were part of a recv() winsock procedure, fyi  */
>     char *bufdata = malloc(sizeof(char)* (TCP_BUFSIZE / 2));
>     int datasize = TCP_BUFSIZE;
>     int numbytes = 0;
>
>     char *alldata = malloc(sizeof(char)*datasize);
>     memset(alldata,0,strlen(alldata));

This is dangerous.  This can attempt to set a random amount of memory
to zero.  The problem is with the use of strlen to determine the
number of bytes.  If the memory block that alldata doesn't contain a
'\0' then strlen will return with a size larger then what was passed
to malloc.

If you want alldata to be initialized try calloc, or use memset with a
fixed size.
If you want alldata to be the empty string, try this instead.
alldata[0] = '\0';

>
>     char *teststring = "Just testing realloc and stuff, long string
> blah blah blah.";
>     char *testptr, *tempdata;
>     int i,tslen = strlen(teststring);
>
>         /* copying teststring to alldata by increments of TCP_BUFSIZE ,
>         verifying that alldata doesnt get overflowed in the process. */
>     for (testptr = teststring,i = 0;i < tslen;testptr = testptr +
> TCP_BUFSIZE,i += 2) {
>                 alldata = strncat(alldata,testptr,TCP_BUFSIZE);

This causes a buffer overflow on the first iteration of the loop.
strncat ALWAYS appends a null, ie. this appends 3 characters each
time, alldata was initially malloc'd for 2 char.

>                 if (strlen(alldata) >= datasize) {
>                     datasize *= 2;
>                         /* Should check realloc result, but lets keep the testcase simple. */
>                     alldata = realloc(alldata,datasize);
>                 }

This buffer size check should be done before you copy data into the
buffer, lest you overflow the buffer first and then grow it in size.
In general the null terminator of the string is usually just outside
the memory space allocated for the buffer.

>                 printf("String: %s\n",alldata);
>     }
>     printf("\nFinal Result: %s",alldata);
>     return 0;
> }
>
> ---- /Code ----

Depending upon what buffer size malloc returns and what data you are
stomping on this may or may not run as expected.

--
Frodak

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Problem reports:       http://cygwin.com/problems.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]