X-Spam-Check-By: sourceware.org
Message-ID: <9ae083a20708081035v46978c7bo96474be59e4205a7@mail.gmail.com>
Date: Wed, 8 Aug 2007 13:35:07 -0400
From: "Frodak Baksik" <frodak17 AT gmail DOT com>
To: cygwin AT cygwin DOT com
Subject: Re: Using malloc/realloc along with gdb: heap overflows
In-Reply-To: <efacbe410708080926u6365eb63v381b7fa5d7972b03@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
References: <efacbe410708080926u6365eb63v381b7fa5d7972b03 AT mail DOT gmail DOT com>
X-IsSubscribed: yes
Mailing-List: contact cygwin-help AT cygwin DOT com; run by ezmlm
Precedence: bulk
List-Id: <cygwin.cygwin.com>
List-Unsubscribe: <mailto:cygwin-unsubscribe-archive-cygwin=delorie DOT com AT cygwin DOT com>
List-Subscribe: <mailto:cygwin-subscribe AT cygwin DOT com>
List-Archive: <http://sourceware.org/ml/cygwin/>
List-Post: <mailto:cygwin AT cygwin DOT com>
List-Help: <mailto:cygwin-help AT cygwin DOT com>, <http://sourceware.org/ml/#faqs>
Sender: cygwin-owner AT cygwin DOT com
Mail-Followup-To: cygwin AT cygwin DOT com
Delivered-To: mailing list cygwin AT cygwin DOT com

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/