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 <ivansprundel@ioactive.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Reviewed-by: Matthieu Herrb <matthieu.herrb@laas.fr>

1 files changed, 15 insertions, 9 deletions

diff --git a/src/Xrm.c b/src/Xrm.c
index 3e29ab0f..2c0c324c 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)