HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Is this program susceptible to a buffer overflow exploit?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisoverflowprogramsusceptibleexploitbuffer

Problem

I use it in a number of embedded devices. It loads the query parameters from an HTTP GET/POST request and prints them to stdout in FORM_key=value format which then get set as an environment variable.

Source:

```
char *
FP_strdup (char *string)
{
char *result;

if (string == NULL)
return NULL;

if ((result = (char *) malloc (strlen (string) + 1)) == NULL) {
fprintf (stderr, "proccgi -- out of memory dupping %d bytes\n",
(int) strlen (string));
return NULL;
}

strcpy (result, string);
return result;
}

/*
* Read CGI input
*/

char *
LoadInput (void)
{
char result, method, *p;
int length, ts;

if ((method = getenv ("REQUEST_METHOD")) == NULL) {
return NULL;
}

if (strcmp (method, "GET") == 0) {
if ((p = getenv ("QUERY_STRING")) == NULL)
return NULL;
else
result = FP_strdup (p);
}
else if (strcmp (method, "POST") == 0) {
if ((length = atoi (getenv ("CONTENT_LENGTH"))) == 0)
return NULL;

if ((result = malloc (length + 1)) == NULL) {
fprintf (stderr, "proccgi -- out of memory allocating %d bytes\n",
length);
return NULL;
}

if ((ts = fread (result, sizeof (char), length, stdin)) ='0'&&ptr1='0'&&ptr1instring && isspace(*(ptr2-1)))
ptr2--;

*ptr2 = '\0';

return instring;
}

/*
* break into attribute/value pair. Mustn't use strtok, which is
* already used one level below. We assume that the attribute doesn't
* have any special characters.
*/

void
HandleString (char *input)
{
char data, ptr, *p2;

if (input == NULL) {
return;
}

data = FP_strdup (input);
ptr = ParseString (data);

/*
* Security:
*
* only accept all-alphanumeric attributes, and don't accept empty
* values
*/

if (!isalpha(ptr) && ptr != '_') {free (data); return;}
ptr++;
while (isalnum(ptr) || ptr == '_') ptr++;
if (*ptr != '=') {free (data); return;}

*ptr = '\0';
p2 = ptr+1;

fprintf (stdout, "FORM_%s=\"", data);

Solution

Your program parses the Content-Length header and stores its value in an int called length:

length = atoi (getenv ("CONTENT_LENGTH")))


Then it increments this value and uses it as the argument of a call to malloc():

result = malloc (length + 1))


Suppose I were to post 232−1 bytes of data to your program (about 4 GB in other words). If it's running in a 32-bit environment, the value of length + 1 will overflow to zero. The subsequent call to malloc() will probably succeed, but the call to fread() will crash the program immediately.

Code Snippets

length = atoi (getenv ("CONTENT_LENGTH")))
result = malloc (length + 1))

Context

StackExchange Code Review Q#79019, answer score: 3

Revisions (0)

No revisions yet.