[PATCH xserver/hw/xfree86/parser/scan.c] potential buffer overrun

vdb at picaros.org vdb at picaros.org
Sat Sep 17 09:58:18 PDT 2011


This patch fixes a potential buffer overrun in xf86addComment().  The 
overrun occurs if the comment string to add doesn't start with a 
leading '#'.  

The bug can be seen from the code assuming 
 curlen > 0 && eol_seen == 0 && iscomment == 0 :

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

<...>

	str = add;
->	iscomment = 0;
	while (*str) {
	    if (*str != ' ' && *str != '\t')
		break;
	    ++str;
	}
->	iscomment = (*str == '#');

        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");

The patch below is just a bugfix and tries to stay as close as 
possible to the original code.  See also: 
 http://lists.x.org/archives/xorg-devel/2011-August/024775.html .

Signed-off-by: Servaas Vandenberghe <vdb at picaros.org>
diff --git a/hw/xfree86/parser/scan.c b/hw/xfree86/parser/scan.c
index 1cff3bc..b188503 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;
@@ -1108,7 +1108,6 @@ xf86addComment(char *cur, char *add)
 		curlen = 0;
 
 	str = add;
-	iscomment = 0;
 	while (*str) {
 	    if (*str != ' ' && *str != '\t')
 		break;
@@ -1118,14 +1117,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++] = '#';


More information about the xorg-devel mailing list