summaryrefslogtreecommitdiff
authorAlan Coopersmith <alan.coopersmith@oracle.com>2013-03-02 20:01:39 (GMT)
committer Alan Coopersmith <alan.coopersmith@oracle.com>2013-05-10 01:59:52 (GMT)
commit236b603d235dc264d1c6250dca09c745458a9088 (patch) (side-by-side diff)
treea80d778be29d154b3da18286c877a790a4506f82
parent076428918e6c35f66b9b55c3fa097ff06496d155 (diff)
downloadlibX11-236b603d235dc264d1c6250dca09c745458a9088.zip
libX11-236b603d235dc264d1c6250dca09c745458a9088.tar.gz
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>
Diffstat (more/less context) (ignore whitespace changes)
-rw-r--r--src/Xrm.c24
1 files 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)