From 820d9040f50a8440741b3aefbc069a3ad81e824e Mon Sep 17 00:00:00 2001 From: Servaas Vandenberghe Date: Wed, 31 Aug 2011 07:06:49 +0200 Subject: 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 Reviewed-by: Peter Hutterer [whot: added buffer overflow test case] Signed-off-by: Peter Hutterer --- hw/xfree86/parser/scan.c | 17 +++++++++++++---- test/xfree86.c | 26 ++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/hw/xfree86/parser/scan.c b/hw/xfree86/parser/scan.c index 1cff3bc5c..99b325717 100644 --- a/hw/xfree86/parser/scan.c +++ b/hw/xfree86/parser/scan.c @@ -1093,7 +1093,7 @@ char * xf86addComment(char *cur, char *add) { char *str; - int len, curlen, iscomment, hasnewline = 0, endnewline; + int len, curlen, iscomment, hasnewline = 0, insnewline, endnewline; if (add == NULL || add[0] == '\0') return cur; @@ -1118,14 +1118,23 @@ 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) + 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; cur = str; - if (eol_seen || (curlen && !hasnewline)) + if (insnewline) cur[curlen++] = '\n'; if (!iscomment) cur[curlen++] = '#'; diff --git a/test/xfree86.c b/test/xfree86.c index 7012e90c3..448aa915e 100644 --- a/test/xfree86.c +++ b/test/xfree86.c @@ -29,6 +29,7 @@ #include "xf86.h" +#include "xf86Parser.h" static void xfree86_option_list_duplicate(void) @@ -73,9 +74,34 @@ xfree86_option_list_duplicate(void) 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) { xfree86_option_list_duplicate(); + xfree86_add_comment(); return 0; } -- cgit v1.2.3