From 236b603d235dc264d1c6250dca09c745458a9088 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 2 Mar 2013 12:01:39 -0800 Subject: 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 Signed-off-by: Alan Coopersmith Reviewed-by: Matthieu Herrb --- src/Xrm.c | 24 +++++++++++++++--------- 1 file 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) -- cgit v1.2.3