xfree86: fix potential buffer overflow

The patch below fixes a potential buffer overflow in xf86addComment().
This occurs if  curlen > 0 && eol_seen == 0 && iscomment == 0 , as
follows from the code:

char *xf86addComment(char *cur, char *add)

<...>

        len = strlen(add);
        endnewline = add[len - 1] == '\n';
        len +=  1 + iscomment + (!hasnewline) + (!endnewline) + eol_seen;

        if ((str = realloc(cur, len + curlen)) == NULL)
                return cur;

        cur = str;

        if (eol_seen || (curlen && !hasnewline))
                cur[curlen++] = '\n';
        if (!iscomment)
                cur[curlen++] = '#';
        strcpy(cur + curlen, add);
        if (!endnewline)
                strcat(cur, "\n");

Signed-off-by: Servaas Vandenberghe <vdb@picaros.org>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>

[whot: added buffer overflow test case]

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
This commit is contained in:
Servaas Vandenberghe 2011-08-31 07:06:49 +02:00 committed by Peter Hutterer
parent 63e87b8639
commit 820d9040f5
2 changed files with 39 additions and 4 deletions

View File

@ -1093,7 +1093,7 @@ char *
xf86addComment(char *cur, char *add) xf86addComment(char *cur, char *add)
{ {
char *str; char *str;
int len, curlen, iscomment, hasnewline = 0, endnewline; int len, curlen, iscomment, hasnewline = 0, insnewline, endnewline;
if (add == NULL || add[0] == '\0') if (add == NULL || add[0] == '\0')
return cur; return cur;
@ -1118,14 +1118,23 @@ xf86addComment(char *cur, char *add)
len = strlen(add); len = strlen(add);
endnewline = add[len - 1] == '\n'; endnewline = add[len - 1] == '\n';
len += 1 + iscomment + (!hasnewline) + (!endnewline) + eol_seen;
if ((str = realloc(cur, len + curlen)) == NULL) insnewline = eol_seen || (curlen && !hasnewline);
if (insnewline)
len++;
if (!iscomment)
len++;
if (!endnewline)
len++;
/* Allocate + 1 char for '\0' terminator. */
str = realloc(cur, curlen + len + 1);
if (!str)
return cur; return cur;
cur = str; cur = str;
if (eol_seen || (curlen && !hasnewline)) if (insnewline)
cur[curlen++] = '\n'; cur[curlen++] = '\n';
if (!iscomment) if (!iscomment)
cur[curlen++] = '#'; cur[curlen++] = '#';

View File

@ -29,6 +29,7 @@
#include "xf86.h" #include "xf86.h"
#include "xf86Parser.h"
static void static void
xfree86_option_list_duplicate(void) xfree86_option_list_duplicate(void)
@ -73,9 +74,34 @@ xfree86_option_list_duplicate(void)
assert(a && b); assert(a && b);
} }
static void
xfree86_add_comment(void)
{
char *current = NULL, *comment;
char compare[1024] = {0};
comment = "# foo";
current = xf86addComment(current, comment);
strcpy(compare, comment);
strcat(compare, "\n");
assert(!strcmp(current, compare));
/* this used to overflow */
strcpy(current, "\n");
comment = "foobar\n";
current = xf86addComment(current, comment);
strcpy(compare, "\n#");
strcat(compare, comment);
assert(!strcmp(current, compare));
free(current);
}
int main(int argc, char** argv) int main(int argc, char** argv)
{ {
xfree86_option_list_duplicate(); xfree86_option_list_duplicate();
xfree86_add_comment();
return 0; return 0;
} }