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>

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;
 }