Chris Evans discovered numerous vulnerabilities in the xpdf package:

Multiple integer overflow issues affecting xpdf.
These can result in writing an arbitrary byte to an attacker controlled
location which probably could lead to arbitrary code execution.
CAN-2004-0888

Multiple integer overflow issues.
These can result in DoS or possibly arbitrary code execution.
CAN-2004-0889

Chris also discovered issues with infinite loop logic error.
This commit is contained in:
brad 2004-10-23 02:24:36 +00:00
parent 26f299c152
commit 31ca292e2d
3 changed files with 299 additions and 1 deletions

View File

@ -1,8 +1,9 @@
# $OpenBSD: Makefile,v 1.42 2004/07/30 01:19:28 naddy Exp $
# $OpenBSD: Makefile,v 1.43 2004/10/23 02:24:36 brad Exp $
COMMENT= "PDF viewer for X"
DISTNAME= xpdf-3.00
PKGNAME= ${DISTNAME}p1
CATEGORIES= textproc x11
MASTER_SITES= ftp://ftp.foolabs.com/pub/xpdf/

View File

@ -0,0 +1,31 @@
$OpenBSD: patch-xpdf_Catalog_cc,v 1.1 2004/10/23 02:24:37 brad Exp $
--- xpdf/Catalog.cc.orig Wed Jan 21 20:26:45 2004
+++ xpdf/Catalog.cc Fri Oct 22 21:54:48 2004
@@ -64,6 +64,15 @@ Catalog::Catalog(XRef *xrefA) {
}
pagesSize = numPages0 = (int)obj.getNum();
obj.free();
+ // The gcc doesnt optimize this away, so this check is ok,
+ // even if it looks like a pagesSize != pagesSize check
+ if (pagesSize*sizeof(Page *)/sizeof(Page *) != pagesSize ||
+ pagesSize*sizeof(Ref)/sizeof(Ref) != pagesSize) {
+ error(-1, "Invalid 'pagesSize'");
+ ok = gFalse;
+ return;
+ }
+
pages = (Page **)gmalloc(pagesSize * sizeof(Page *));
pageRefs = (Ref *)gmalloc(pagesSize * sizeof(Ref));
for (i = 0; i < pagesSize; ++i) {
@@ -191,6 +200,11 @@ int Catalog::readPageTree(Dict *pagesDic
}
if (start >= pagesSize) {
pagesSize += 32;
+ if (pagesSize*sizeof(Page *)/sizeof(Page *) != pagesSize ||
+ pagesSize*sizeof(Ref)/sizeof(Ref) != pagesSize) {
+ error(-1, "Invalid 'pagesSize' parameter.");
+ goto err3;
+ }
pages = (Page **)grealloc(pages, pagesSize * sizeof(Page *));
pageRefs = (Ref *)grealloc(pageRefs, pagesSize * sizeof(Ref));
for (j = pagesSize - 32; j < pagesSize; ++j) {

View File

@ -0,0 +1,266 @@
$OpenBSD: patch-xpdf_XRef_cc,v 1.1 2004/10/23 02:24:37 brad Exp $
--- xpdf/XRef.cc.orig Wed Jan 21 20:26:45 2004
+++ xpdf/XRef.cc Fri Oct 22 21:54:48 2004
@@ -96,7 +96,7 @@ ObjectStream::ObjectStream(XRef *xref, i
}
nObjects = obj1.getInt();
obj1.free();
- if (nObjects == 0) {
+ if (nObjects <= 0) {
goto err1;
}
@@ -106,7 +106,14 @@ ObjectStream::ObjectStream(XRef *xref, i
}
first = obj1.getInt();
obj1.free();
+ if (first < 0) {
+ goto err1;
+ }
+ if (nObjects*sizeof(int)/sizeof(int) != nObjects) {
+ error(-1, "Invalid 'nObjects'");
+ goto err1;
+ }
objs = new Object[nObjects];
objNums = (int *)gmalloc(nObjects * sizeof(int));
offsets = (int *)gmalloc(nObjects * sizeof(int));
@@ -130,6 +137,12 @@ ObjectStream::ObjectStream(XRef *xref, i
offsets[i] = obj2.getInt();
obj1.free();
obj2.free();
+ if (objNums[i] < 0 || offsets[i] < 0 ||
+ (i > 0 && offsets[i] < offsets[i-1])) {
+ delete parser;
+ gfree(offsets);
+ goto err1;
+ }
}
while (str->getChar() != EOF) ;
delete parser;
@@ -369,10 +382,20 @@ GBool XRef::readXRefTable(Parser *parser
}
n = obj.getInt();
obj.free();
+ if (first < 0 || n < 0 || first + n < 0) {
+ goto err1;
+ }
if (first + n > size) {
for (newSize = size ? 2 * size : 1024;
- first + n > newSize;
+ first + n > newSize && newSize > 0;
newSize <<= 1) ;
+ if (newSize*sizeof(XRefEntry)/sizeof(XRefEntry) != newSize) {
+ error(-1, "Invalid 'obj' parameters'");
+ return gFalse;
+ }
+ if (newSize < 0) {
+ goto err1;
+ }
entries = (XRefEntry *)grealloc(entries, newSize * sizeof(XRefEntry));
for (i = size; i < newSize; ++i) {
entries[i].offset = 0xffffffff;
@@ -443,7 +466,7 @@ GBool XRef::readXRefTable(Parser *parser
// check for an 'XRefStm' key
if (obj.getDict()->lookup("XRefStm", &obj2)->isInt()) {
- pos2 = obj2.getInt();
+ pos2 = (Guint)obj2.getInt();
readXRef(&pos2);
if (!ok) {
goto err1;
@@ -474,7 +497,14 @@ GBool XRef::readXRefStream(Stream *xrefS
}
newSize = obj.getInt();
obj.free();
+ if (newSize < 0) {
+ goto err1;
+ }
if (newSize > size) {
+ if (newSize * sizeof(XRefEntry)/sizeof(XRefEntry) != newSize) {
+ error(-1, "Invalid 'size' parameter.");
+ return gFalse;
+ }
entries = (XRefEntry *)grealloc(entries, newSize * sizeof(XRefEntry));
for (i = size; i < newSize; ++i) {
entries[i].offset = 0xffffffff;
@@ -494,6 +524,9 @@ GBool XRef::readXRefStream(Stream *xrefS
}
w[i] = obj2.getInt();
obj2.free();
+ if (w[i] < 0 || w[i] > 4) {
+ goto err1;
+ }
}
obj.free();
@@ -513,13 +546,14 @@ GBool XRef::readXRefStream(Stream *xrefS
}
n = obj.getInt();
obj.free();
- if (!readXRefStreamSection(xrefStr, w, first, n)) {
+ if (first < 0 || n < 0 ||
+ !readXRefStreamSection(xrefStr, w, first, n)) {
idx.free();
goto err0;
}
}
} else {
- if (!readXRefStreamSection(xrefStr, w, 0, size)) {
+ if (!readXRefStreamSection(xrefStr, w, 0, newSize)) {
idx.free();
goto err0;
}
@@ -551,10 +585,20 @@ GBool XRef::readXRefStreamSection(Stream
Guint offset;
int type, gen, c, newSize, i, j;
+ if (first + n < 0) {
+ return gFalse;
+ }
if (first + n > size) {
for (newSize = size ? 2 * size : 1024;
- first + n > newSize;
+ first + n > newSize && newSize > 0;
newSize <<= 1) ;
+ if (newSize*sizeof(XRefEntry)/sizeof(XRefEntry) != newSize) {
+ error(-1, "Invalid 'size' inside xref table.");
+ return gFalse;
+ }
+ if (newSize < 0) {
+ return gFalse;
+ }
entries = (XRefEntry *)grealloc(entries, newSize * sizeof(XRefEntry));
for (i = size; i < newSize; ++i) {
entries[i].offset = 0xffffffff;
@@ -585,24 +629,26 @@ GBool XRef::readXRefStreamSection(Stream
}
gen = (gen << 8) + c;
}
- switch (type) {
- case 0:
- entries[i].offset = offset;
- entries[i].gen = gen;
- entries[i].type = xrefEntryFree;
- break;
- case 1:
- entries[i].offset = offset;
- entries[i].gen = gen;
- entries[i].type = xrefEntryUncompressed;
- break;
- case 2:
- entries[i].offset = offset;
- entries[i].gen = gen;
- entries[i].type = xrefEntryCompressed;
- break;
- default:
- return gFalse;
+ if (entries[i].offset == 0xffffffff) {
+ switch (type) {
+ case 0:
+ entries[i].offset = offset;
+ entries[i].gen = gen;
+ entries[i].type = xrefEntryFree;
+ break;
+ case 1:
+ entries[i].offset = offset;
+ entries[i].gen = gen;
+ entries[i].type = xrefEntryUncompressed;
+ break;
+ case 2:
+ entries[i].offset = offset;
+ entries[i].gen = gen;
+ entries[i].type = xrefEntryCompressed;
+ break;
+ default:
+ return gFalse;
+ }
}
}
@@ -664,39 +710,49 @@ GBool XRef::constructXRef() {
// look for object
} else if (isdigit(*p)) {
num = atoi(p);
- do {
- ++p;
- } while (*p && isdigit(*p));
- if (isspace(*p)) {
+ if (num > 0) {
do {
++p;
- } while (*p && isspace(*p));
- if (isdigit(*p)) {
- gen = atoi(p);
+ } while (*p && isdigit(*p));
+ if (isspace(*p)) {
do {
++p;
- } while (*p && isdigit(*p));
- if (isspace(*p)) {
+ } while (*p && isspace(*p));
+ if (isdigit(*p)) {
+ gen = atoi(p);
do {
++p;
- } while (*p && isspace(*p));
- if (!strncmp(p, "obj", 3)) {
- if (num >= size) {
- newSize = (num + 1 + 255) & ~255;
- entries = (XRefEntry *)
- grealloc(entries, newSize * sizeof(XRefEntry));
- for (i = size; i < newSize; ++i) {
- entries[i].offset = 0xffffffff;
- entries[i].type = xrefEntryFree;
+ } while (*p && isdigit(*p));
+ if (isspace(*p)) {
+ do {
+ ++p;
+ } while (*p && isspace(*p));
+ if (!strncmp(p, "obj", 3)) {
+ if (num >= size) {
+ newSize = (num + 1 + 255) & ~255;
+ if (newSize*sizeof(XRefEntry)/sizeof(XRefEntry) != newSize) {
+ error(-1, "Invalid 'obj' parameters.");
+ return gFalse;
+ }
+ if (newSize < 0) {
+ error(-1, "Bad object number");
+ return gFalse;
+ }
+ entries = (XRefEntry *)
+ grealloc(entries, newSize * sizeof(XRefEntry));
+ for (i = size; i < newSize; ++i) {
+ entries[i].offset = 0xffffffff;
+ entries[i].type = xrefEntryFree;
+ }
+ size = newSize;
}
- size = newSize;
+ if (entries[num].type == xrefEntryFree ||
+ gen >= entries[num].gen) {
+ entries[num].offset = pos - start;
+ entries[num].gen = gen;
+ entries[num].type = xrefEntryUncompressed;
+ }
}
- if (entries[num].type == xrefEntryFree ||
- gen >= entries[num].gen) {
- entries[num].offset = pos - start;
- entries[num].gen = gen;
- entries[num].type = xrefEntryUncompressed;
- }
}
}
}
@@ -705,6 +761,10 @@ GBool XRef::constructXRef() {
} else if (!strncmp(p, "endstream", 9)) {
if (streamEndsLen == streamEndsSize) {
streamEndsSize += 64;
+ if (streamEndsSize*sizeof(int)/sizeof(int) != streamEndsSize) {
+ error(-1, "Invalid 'endstream' parameter.");
+ return gFalse;
+ }
streamEnds = (Guint *)grealloc(streamEnds,
streamEndsSize * sizeof(int));
}