diff options
author | Alan Coopersmith <alan.coopersmith@oracle.com> | 2013-03-02 12:01:39 -0800 |
---|---|---|
committer | Alan Coopersmith <alan.coopersmith@oracle.com> | 2013-05-09 18:59:52 -0700 |
commit | 236b603d235dc264d1c6250dca09c745458a9088 (patch) | |
tree | a80d778be29d154b3da18286c877a790a4506f82 | |
parent | 076428918e6c35f66b9b55c3fa097ff06496d155 (diff) |
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>
-rw-r--r-- | src/Xrm.c | 24 |
1 files changed, 15 insertions, 9 deletions
@@ -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) |