[Bug 2809525] Abort on overlong color name

3 files changed, 62 insertions, 60 deletions

diff --git a/ChangeLog b/ChangeLog
index 2626d5c..7f30a4f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2012-03-21  Jan Nijtmans  <nijtmans@users.sf.net>
+
+	* generic/tkColor.c:  [Bug 2809525] Abort on overlong color name.
+	* unix/tkUnixColor.c:
+
 2012-03-18  Jan Nijtmans  <nijtmans@users.sf.net>
 
 	* xlib/xcolors.c:   [RFE 3503317]: XParseColor speedup
diff --git a/generic/tkColor.c b/generic/tkColor.c
index da2cb9b..cafaaaa 100644
--- a/generic/tkColor.c
+++ b/generic/tkColor.c
@@ -804,55 +804,63 @@ TkDebugColor(
  * since XParseColor already does the right thing */
 Status
 TkParseColor(
-    Display * display,		/* The display */
+    Display *display,		/* The display */
     Colormap map,			/* Color map */
-    const char* spec,     /* String to be parsed */
-    XColor * colorPtr)
+    const char *name,     /* String to be parsed */
+    XColor *color)
 {
-    if (*spec == '#') {
-    char buf[14];
-    buf[0] = '#'; buf[13] = '\0';
-	if (!*(++spec) || !*(++spec) || !*(++spec)) {
-	/* Not at least 3 hex digits, so invalid */
+    if (*name == '#') {
+	char buf[14];
+	buf[0] = '#'; buf[13] = '\0';
+	if (!*(++name) || !*(++name) || !*(++name)) {
+	    /* Not at least 3 hex digits, so invalid */
 	return 0;
-	} else if (!*(++spec)) {
-	/* Exactly 3 hex digits */
-	buf[9] = buf[10] = buf[11] = buf[12] = *(--spec);
-	buf[5] = buf[6] = buf[7] = buf[8] = *(--spec);
-	buf[1] = buf[2] = buf[3] = buf[4] = *(--spec);
-	spec = buf;
-	} else if (!*(++spec)	|| !*(++spec)) {
-	/* Not at least 6 hex digits, so invalid */
-	return 0;
-	} else if (!*(++spec)) {
-	/* Exactly 6 hex digits */
-	buf[10] = buf[12] = *(--spec);
-	buf[9] = buf[11] = *(--spec);
-	buf[6] = buf[8] = *(--spec);
-	buf[5] = buf[7] = *(--spec);
-	buf[2] = buf[4] = *(--spec);
-	buf[1] = buf[3] = *(--spec);
-	spec = buf;
-	} else if (!*(++spec) || !*(++spec)) {
-	/* Not at least 9 hex digits, so invalid */
-	return 0;
-	} else if (!*(++spec)) {
-	/* Exactly 9 hex digits */
-	buf[11] = *(--spec);
-	buf[10] = *(--spec);
-	buf[9] = buf[12] = *(--spec);
-	buf[7] = *(--spec);
-	buf[6] = *(--spec);
-	buf[5] = buf[8] = *(--spec);
-	buf[3] = *(--spec);
-	buf[2] = *(--spec);
-	buf[1] = buf[4] = *(--spec);
-	spec = buf;
+	} else if (!*(++name)) {
+	    /* Exactly 3 hex digits */
+	    buf[9] = buf[10] = buf[11] = buf[12] = *(--name);
+	    buf[5] = buf[6] = buf[7] = buf[8] = *(--name);
+	    buf[1] = buf[2] = buf[3] = buf[4] = *(--name);
+	    name = buf;
+	} else if (!*(++name)	|| !*(++name)) {
+	    /* Not at least 6 hex digits, so invalid */
+	    return 0;
+	} else if (!*(++name)) {
+	    /* Exactly 6 hex digits */
+	    buf[10] = buf[12] = *(--name);
+	    buf[9] = buf[11] = *(--name);
+	    buf[6] = buf[8] = *(--name);
+	    buf[5] = buf[7] = *(--name);
+	    buf[2] = buf[4] = *(--name);
+	    buf[1] = buf[3] = *(--name);
+	    name = buf;
+	} else if (!*(++name) || !*(++name)) {
+	    /* Not at least 9 hex digits, so invalid */
+	    return 0;
+	} else if (!*(++name)) {
+	    /* Exactly 9 hex digits */
+	    buf[11] = *(--name);
+	    buf[10] = *(--name);
+	    buf[9] = buf[12] = *(--name);
+	    buf[7] = *(--name);
+	    buf[6] = *(--name);
+	    buf[5] = buf[8] = *(--name);
+	    buf[3] = *(--name);
+	    buf[2] = *(--name);
+	    buf[1] = buf[4] = *(--name);
+	    name = buf;
+	} else if (!*(++name) || !*(++name) || *(++name)) {
+	    /* Not exactly 12 hex digits, so invalid */
+	    return 0;
 	} else {
-	    spec -= 10;
+	    name -= 13;
+	}
+    } else {
+	if (strlen(name) > 99) {
+	    /* Don't bother to parse this. [Bug 2809525]*/
+	    return 0;
 	}
     }
-    return XParseColor(display, map, spec, colorPtr);
+    return XParseColor(display, map, name, color);
 }
 #endif /* __WIN32__ */
 /*
diff --git a/unix/tkUnixColor.c b/unix/tkUnixColor.c
index d3dba23..a455118 100644
--- a/unix/tkUnixColor.c
+++ b/unix/tkUnixColor.c
@@ -126,20 +126,6 @@ TkpGetColor(
     Colormap colormap = Tk_Colormap(tkwin);
     XColor color;
     TkColor *tkColPtr;
-    char buf[100];
-    unsigned len = strlen(name);
-
-    /*
-     * Make sure that we never exceed a reasonable length of color name. A
-     * good maximum length is 99, arbitrary, but larger than any known color
-     * name. [Bug 2809525]
-     */
-
-    if (len > 99) {
-	len = 99;
-    }
-    memcpy(buf, name, len);
-    buf[len] = '\0';
 
     /*
      * Map from the name to a pixel value. Call XAllocNamedColor rather than
@@ -150,7 +136,10 @@ TkpGetColor(
     if (*name != '#') {
 	XColor screen;
 
-	if (XAllocNamedColor(display, colormap, buf, &screen, &color) != 0) {
+	if (strlen(name) > 99) {
+	/* Don't bother to parse this. [Bug 2809525]*/
+	return (TkColor *) NULL;
+    } else if (XAllocNamedColor(display, colormap, name, &screen, &color) != 0) {
 	    DeleteStressedCmap(display, colormap);
 	} else {
 	    /*
@@ -160,13 +149,13 @@ TkpGetColor(
 	     * approximation to the desired color.
 	     */
 
-	    if (XLookupColor(display, colormap, buf, &color, &screen) == 0) {
+	    if (XLookupColor(display, colormap, name, &color, &screen) == 0) {
 		return NULL;
 	    }
 	    FindClosestColor(tkwin, &screen, &color);
 	}
     } else {
-	if (TkParseColor(display, colormap, buf, &color) == 0) {
+	if (TkParseColor(display, colormap, name, &color) == 0) {
 	    return NULL;
 	}
 	if (XAllocColor(display, colormap, &color) != 0) {