Protect against overly long strings

Checking against upper limit of USHRT_MAX must happen before truncating
size_t to int. On 64 bit systems with strings larger than 2 GB this
could otherwise lead to negative ints or ints smaller than USHRT_MAX.

In XParseColor this could lead to out of boundary access with strings
starting with a # (color sequence). A modulo 12 operation is performed
to validate the string length, but with an overflown length, the for
loop would eventually read behind terminating '\0' character.

Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>

4 files changed, 15 insertions, 15 deletions

diff --git a/src/Font.c b/src/Font.c
index d4314e26..ae2cc1a0 100644
--- a/src/Font.c
+++ b/src/Font.c
@@ -656,7 +656,7 @@ int _XF86LoadQueryLocaleFont(
    XFontStruct **xfp,
    Font *fidp)
 {
-    int l;
+    size_t l;
     const char *charset, *p;
     char buf[256];
     XFontStruct *fs;
@@ -664,7 +664,7 @@ int _XF86LoadQueryLocaleFont(
 
     if (!name)
 	return 0;
-    l = (int) strlen(name);
+    l = strlen(name);
     if (l < 2 || name[l - 1] != '*' || name[l - 2] != '-' || l >= USHRT_MAX)
 	return 0;
     charset = NULL;
@@ -677,11 +677,11 @@ int _XF86LoadQueryLocaleFont(
 	charset = "ISO8859-1";
 	p = charset + 7;
     }
-    if (l - 2 - (p - charset) < 0)
+    if (l - 2 < p - charset)
 	return 0;
     if (_XlcNCompareISOLatin1(name + l - 2 - (p - charset), charset, p - charset))
 	return 0;
-    if (strlen(p + 1) + (size_t) l - 1 >= sizeof(buf) - 1)
+    if (strlen(p + 1) + l - 1 >= sizeof(buf) - 1)
 	return 0;
     strcpy(buf, name);
     strcpy(buf + l - 1, p + 1);
diff --git a/src/LookupCol.c b/src/LookupCol.c
index 5df09fb4..3b53bf87 100644
--- a/src/LookupCol.c
+++ b/src/LookupCol.c
@@ -41,7 +41,7 @@ XLookupColor (
 	XColor *def,
 	XColor *scr)
 {
-	register int n;
+	register size_t n;
 	xLookupColorReply reply;
 	register xLookupColorReq *req;
 	XcmsCCC ccc;
@@ -49,7 +49,7 @@ XLookupColor (
 
 	if (spec == NULL)
 		return 0;
-	n = (int) strlen (spec);
+	n = strlen (spec);
 	if (n >= USHRT_MAX)
             return 0;
 #ifdef XCMS
diff --git a/src/ParseCol.c b/src/ParseCol.c
index ff5a264e..fcf90692 100644
--- a/src/ParseCol.c
+++ b/src/ParseCol.c
@@ -40,14 +40,14 @@ XParseColor (
 	_Xconst char *spec,
 	XColor *def)
 {
-	register int n, i;
+	register size_t n, i;
 	int r, g, b;
 	char c;
 	XcmsCCC ccc;
 	XcmsColor cmsColor;
 
-	if (!spec) return(0);
-	n = (int) strlen (spec);
+        if (!spec) return(0);
+	n = strlen (spec);
 	if (n >= USHRT_MAX)
             return(0);
 	if (*spec == '#') {
@@ -64,7 +64,7 @@ XParseColor (
 		r = g;
 		g = b;
 		b = 0;
-		for (i = n; --i >= 0; ) {
+		for (i = 0; i < n; i++) {
 		    c = *spec++;
 		    b <<= 4;
 		    if (c >= '0' && c <= '9')
@@ -122,7 +122,7 @@ XParseColor (
 	    LockDisplay(dpy);
 	    GetReq (LookupColor, req);
 	    req->cmap = cmap;
-	    req->nbytes = (CARD16) (n = (int) strlen(spec));
+	    req->nbytes = (CARD16) (n = strlen(spec));
 	    req->length += (n + 3) >> 2;
 	    Data (dpy, spec, (long)n);
 	    if (!_XReply (dpy, (xReply *) &reply, 0, xTrue)) {
diff --git a/src/SetFPath.c b/src/SetFPath.c
index b60498df..6ac546f7 100644
--- a/src/SetFPath.c
+++ b/src/SetFPath.c
@@ -38,7 +38,7 @@ XSetFontPath (
     char **directories,
     int ndirs)
 {
-	register int n = 0;
+	register size_t n = 0;
 	register int i;
 	register int nbytes;
 	char *p;
@@ -49,7 +49,7 @@ XSetFontPath (
 	GetReq (SetFontPath, req);
 	req->nFonts = ndirs;
 	for (i = 0; i < ndirs; i++) {
-		n = (int) ((size_t) n + (safestrlen (directories[i]) + 1));
+		n = n + (safestrlen (directories[i]) + 1);
 		if (n >= USHRT_MAX) {
 			UnlockDisplay(dpy);
 			SyncHandle();
@@ -65,9 +65,9 @@ XSetFontPath (
 		char	*tmp = p;
 
 		for (i = 0; i < ndirs; i++) {
-			register int length = (int) safestrlen (directories[i]);
+			size_t length = safestrlen (directories[i]);
 			*p = length;
-			memcpy (p + 1, directories[i], (size_t)length);
+			memcpy (p + 1, directories[i], length);
 			p += length + 1;
 		}
 		Data (dpy, tmp, nbytes);