From 5004f53889bf65aa9e78cea7a01a51948839dce3 Mon Sep 17 00:00:00 2001

From: Alan Coopersmith <[email protected]>

Date: Sat, 2 Mar 2013 12:01:39 -0800

Subject: [PATCH:libX11 22/38] Unbounded recursion in GetDatabase() when

 parsing include files [CVE-2013-2004 1/2]

GetIncludeFile() can call GetDatabase() which can call GetIncludeFile()

which can call GetDatabase() which can call GetIncludeFile() ....

eventually causing recursive stack overflow and crash.

Easily reproduced with a resource file that #includes itself.

Limit is set to a include depth of 100 files, which should be enough

for all known use cases, but could be adjusted later if necessary.

Reported-by: Ilja Van Sprundel <[email protected]>

Signed-off-by: Alan Coopersmith <[email protected]>

Reviewed-by: Matthieu Herrb <[email protected]>

---

 src/Xrm.c |   24 +++++++++++++++---------

 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/Xrm.c b/src/Xrm.c

index 3e29ab0..2c0c324 100644

--- a/src/Xrm.c

+++ b/src/Xrm.c

@@ -1088,13 +1088,15 @@ static void GetIncludeFile(

     XrmDatabase db,

     _Xconst char *base,

     _Xconst char *fname,

-    int fnamelen);

+    int fnamelen,

+    int depth);

 static void GetDatabase(

     XrmDatabase db,

     _Xconst char *str,

     _Xconst char *filename,

-    Bool doall)

+    Bool doall,

+    int depth)

 {

     char *rhs;

     char *lhs, lhs_s[DEF_BUFF_SIZE];

@@ -1204,7 +1206,8 @@ static void GetDatabase(

 		    } while (c != '"' && !is_EOL(bits));

 		    /* must have an ending " */

 		    if (c == '"')

-			GetIncludeFile(db, filename, fname, str - len - fname);

+			GetIncludeFile(db, filename, fname, str - len - fname,

+			    depth);

 		}

 	    }

 	    /* spin to next newline */

@@ -1545,7 +1548,7 @@ XrmPutLineResource(

 {

     if (!*pdb) *pdb = NewDatabase();

     _XLockMutex(&(*pdb)->linfo);

-    GetDatabase(*pdb, line, (char *)NULL, False);

+    GetDatabase(*pdb, line, (char *)NULL, False, 0);

     _XUnlockMutex(&(*pdb)->linfo);

 }

@@ -1557,7 +1560,7 @@ XrmGetStringDatabase(

     db = NewDatabase();

     _XLockMutex(&db->linfo);

-    GetDatabase(db, data, (char *)NULL, True);

+    GetDatabase(db, data, (char *)NULL, True, 0);

     _XUnlockMutex(&db->linfo);

     return db;

 }

@@ -1636,7 +1639,8 @@ GetIncludeFile(

     XrmDatabase db,

     _Xconst char *base,

     _Xconst char *fname,

-    int fnamelen)

+    int fnamelen,

+    int depth)

 {

     int len;

     char *str;

@@ -1644,6 +1648,8 @@ GetIncludeFile(

     if (fnamelen <= 0 || fnamelen >= BUFSIZ)

 	return;

+    if (depth >= MAXDBDEPTH)

+	return;

     if (*fname != '/' && base && (str = strrchr(base, '/'))) {

 	len = str - base + 1;

 	if (len + fnamelen >= BUFSIZ)

@@ -1657,7 +1663,7 @@ GetIncludeFile(

     }

     if (!(str = ReadInFile(realfname)))

 	return;

-    GetDatabase(db, str, realfname, True);

+    GetDatabase(db, str, realfname, True, depth + 1);

     Xfree(str);

 }

@@ -1673,7 +1679,7 @@ XrmGetFileDatabase(

     db = NewDatabase();

     _XLockMutex(&db->linfo);

-    GetDatabase(db, str, filename, True);

+    GetDatabase(db, str, filename, True, 0);

     _XUnlockMutex(&db->linfo);

     Xfree(str);

     return db;

@@ -1697,7 +1703,7 @@ XrmCombineFileDatabase(

     } else

 	db = NewDatabase();

     _XLockMutex(&db->linfo);

-    GetDatabase(db, str, filename, True);

+    GetDatabase(db, str, filename, True, 0);

     _XUnlockMutex(&db->linfo);

     Xfree(str);

     if (!override)

-- 

1.7.9.2

From f94dc4a99aa1845c6a25826e99e11aaa4fff78eb Mon Sep 17 00:00:00 2001

From: Alan Coopersmith <[email protected]>

Date: Sat, 2 Mar 2013 12:39:58 -0800

Subject: [PATCH:libX11 23/38] Unbounded recursion in _XimParseStringFile()

 when parsing include files [CVE-2013-2004 2/2]

parseline() can call _XimParseStringFile() which can call parseline()

which can call _XimParseStringFile() which can call parseline() ....

eventually causing recursive stack overflow and crash.

Limit is set to a include depth of 100 files, which should be enough

for all known use cases, but could be adjusted later if necessary.

Reported-by: Ilja Van Sprundel <[email protected]>

Signed-off-by: Alan Coopersmith <[email protected]>

Reviewed-by: Matthieu Herrb <[email protected]>

---

 modules/im/ximcp/imLcPrs.c |   20 +++++++++++++++++---

 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/modules/im/ximcp/imLcPrs.c b/modules/im/ximcp/imLcPrs.c

index 4e54385..4c7d6f0 100644

--- a/modules/im/ximcp/imLcPrs.c

+++ b/modules/im/ximcp/imLcPrs.c

@@ -56,6 +56,8 @@ extern int _Xmbstoutf8(

     int		len

 );

+static void parsestringfile(FILE *fp, Xim im, int depth);

+

 /*

  *	Parsing File Format:

  *

@@ -423,7 +425,8 @@ static int

 parseline(

     FILE *fp,

     Xim   im,

-    char* tokenbuf)

+    char* tokenbuf,

+    int   depth)

 {

     int token;

     DTModifier modifier_mask;

@@ -470,11 +473,13 @@ parseline(

                 goto error;

             if ((filename = TransFileName(im, tokenbuf)) == NULL)

                 goto error;

+            if (++depth > 100)

+                goto error;

             infp = _XFopenFile(filename, "r");

                 Xfree(filename);

             if (infp == NULL)

                 goto error;

-            _XimParseStringFile(infp, im);

+            parsestringfile(infp, im, depth);

             fclose(infp);

             return (0);

 	} else if ((token == KEY) && (strcmp("None", tokenbuf) == 0)) {

@@ -668,6 +673,15 @@ _XimParseStringFile(

     FILE *fp,

     Xim   im)

 {

+    parsestringfile(fp, im, 0);

+}

+

+static void

+parsestringfile(

+    FILE *fp,

+    Xim   im,

+    int   depth)

+{

     char tb[8192];

     char* tbp;

     struct stat st;

@@ -678,7 +692,7 @@ _XimParseStringFile(

 	else tbp = malloc (size);

 	if (tbp != NULL) {

-	    while (parseline(fp, im, tbp) >= 0) {}

+	    while (parseline(fp, im, tbp, depth) >= 0) {}

 	    if (tbp != tb) free (tbp);

 	}

     }

-- 

1.7.9.2