Hi Frank, hi Florian! Frank Küster [2005-12-08 13:17 +0100]: > Martin Pitt <martin.pitt@canonical.com> wrote: > > > Hi! > > > > I'm currently preparing Ubuntu security updates for these issues, and > > I noticed that the upstream provided patch is wrong. I sent the mail > > below to upstream (and some others). > > > > Can you please check that you indeed fixed (tetex-bin)/will fix > > (poppler) DCTStream::readProgressiveSOF(), too? > [...] > > It seems that the patch linked from these advisories [1] is a little > > bit flawed: it checks numComps twice in DCTStream::readBaselineSOF(), > > but does not check it in DCTStream::readProgressiveSOF(). > > We have the same flaw in our upload. Would you be so kind and check the > updated patch at > > http://svn.debian.org/wsvn/pkg-tetex/tetex-bin/trunk/debian/patches/patch-CVE-2005-3191+2+3?op=file&rev=0&sc=0 After discovering that the same flawed multiplication is also present in upstream's other two patches, I decided to completely rework the patch. I attach the debdiff with separated out changelog. Florian, maybe you can peer-review the patch? Thanks! Martin -- Martin Pitt http://www.piware.de Ubuntu Developer http://www.ubuntu.com Debian Developer http://www.debian.org In a world without walls and fences, who needs Windows and Gates?
* SECURITY UPDATE: Multiple integer/buffer overflows in embedded xpdf code.
* Add debian/patches/patch-CVE-2005-3191+2+3.patch:
* xpdf/Stream.cc, DCTStream::readBaselineSOF(),
DCTStream::readProgressiveSOF(), DCTStream::readScanInfo():
- Check numComps for invalid values.
- http://www.idefense.com/application/poi/display?id=342&type=vulnerabilities
- CVE-2005-3191
* xpdf/Stream.cc, StreamPredictor::StreamPredictor():
- Check rowBytes for invalid values.
- http://www.idefense.com/application/poi/display?id=344&type=vulnerabilities
- CVE-2005-3192
* xpdf/JPXStream.cc, JPXStream::readCodestream():
- Check img.nXTiles * img.nYTiles * sizeof for integer overflow.
- http://www.idefense.com/application/poi/display?id=345&type=vulnerabilities
- CVE-2005-3193
diff -u tetex-bin-3.0/debian/patches/series tetex-bin-3.0/debian/patches/series
--- tetex-bin-3.0/debian/patches/series
+++ tetex-bin-3.0/debian/patches/series
@@ -11,0 +12 @@
+patch-CVE-2005-3191+2+3
--- tetex-bin-3.0.orig/debian/patches/patch-CVE-2005-3191+2+3
+++ tetex-bin-3.0/debian/patches/patch-CVE-2005-3191+2+3
@@ -0,0 +1,153 @@
+--- tetex-bin-3.0/libs/xpdf/xpdf/JPXStream.cc
++++ tetex-bin-3.0.new/libs/xpdf/xpdf/JPXStream.cc
+@@ -7,6 +7,7 @@
+ //========================================================================
+
+ #include <aconf.h>
++#include <limits.h>
+
+ #ifdef USE_GCC_PRAGMAS
+ #pragma implementation
+@@ -666,7 +667,7 @@ GBool JPXStream::readCodestream(Guint le
+ int segType;
+ GBool haveSIZ, haveCOD, haveQCD, haveSOT;
+ Guint precinctSize, style;
+- Guint segLen, capabilities, comp, i, j, r;
++ Guint segLen, capabilities, nTiles, comp, i, j, r;
+
+ //----- main header
+ haveSIZ = haveCOD = haveQCD = haveSOT = gFalse;
+@@ -701,8 +702,18 @@ GBool JPXStream::readCodestream(Guint le
+ / img.xTileSize;
+ img.nYTiles = (img.ySize - img.yTileOffset + img.yTileSize - 1)
+ / img.yTileSize;
+- img.tiles = (JPXTile *)gmalloc(img.nXTiles * img.nYTiles *
+- sizeof(JPXTile));
++ // check for overflow before allocating memory
++ if (img.nXTiles <= 0 || img.nYTiles <= 0 ||
++ img.nXTiles >= INT_MAX/img.nYTiles) {
++ error(getPos(), "Bad tile count in JPX SIZ marker segment");
++ return gFalse;
++ }
++ nTiles = img.nXTiles * img.nYTiles;
++ if (nTiles >= INT_MAX/sizeof(JPXTile)) {
++ error(getPos(), "Bad tile count in JPX SIZ marker segment");
++ return gFalse;
++ }
++ img.tiles = (JPXTile *)gmalloc(nTiles * sizeof(JPXTile));
+ for (i = 0; i < img.nXTiles * img.nYTiles; ++i) {
+ img.tiles[i].tileComps = (JPXTileComp *)gmalloc(img.nComps *
+ sizeof(JPXTileComp));
+--- tetex-bin-3.0/libs/xpdf/xpdf/Stream.cc
++++ tetex-bin-3.0.new/libs/xpdf/xpdf/Stream.cc
+@@ -15,6 +15,7 @@
+ #include <stdio.h>
+ #include <stdlib.h>
+ #include <stddef.h>
++#include <limits.h>
+ #ifndef WIN32
+ #include <unistd.h>
+ #endif
+@@ -412,13 +413,28 @@ StreamPredictor::StreamPredictor(Stream
+ width = widthA;
+ nComps = nCompsA;
+ nBits = nBitsA;
++ predLine = NULL;
++ ok = gFalse;
+
++ if (width <= 0 || nComps <= 0 || nBits <= 0 ||
++ nComps >= INT_MAX/nBits ||
++ width >= INT_MAX/nComps/nBits) {
++ return;
++ }
+ nVals = width * nComps;
++ if (nVals + 7 <= 0) {
++ return;
++ }
+ pixBytes = (nComps * nBits + 7) >> 3;
+ rowBytes = ((nVals * nBits + 7) >> 3) + pixBytes;
++ if (rowBytes < 0) {
++ return;
++ }
+ predLine = (Guchar *)gmalloc(rowBytes);
+ memset(predLine, 0, rowBytes);
+ predIdx = rowBytes;
++
++ ok = gTrue;
+ }
+
+ StreamPredictor::~StreamPredictor() {
+@@ -1012,6 +1028,10 @@ LZWStream::LZWStream(Stream *strA, int p
+ FilterStream(strA) {
+ if (predictor != 1) {
+ pred = new StreamPredictor(this, predictor, columns, colors, bits);
++ if (!pred->isOk()) {
++ delete pred;
++ pred = NULL;
++ }
+ } else {
+ pred = NULL;
+ }
+@@ -2897,6 +2917,10 @@ GBool DCTStream::readBaselineSOF() {
+ height = read16();
+ width = read16();
+ numComps = str->getChar();
++ if (numComps <= 0 || numComps > 4) {
++ error(getPos(), "Bad number of components in DCT stream", prec);
++ return gFalse;
++ }
+ if (prec != 8) {
+ error(getPos(), "Bad DCT precision %d", prec);
+ return gFalse;
+@@ -2923,6 +2947,10 @@ GBool DCTStream::readProgressiveSOF() {
+ height = read16();
+ width = read16();
+ numComps = str->getChar();
++ if (numComps <= 0 || numComps > 4) {
++ error(getPos(), "Bad number of components in DCT stream", prec);
++ return gFalse;
++ }
+ if (prec != 8) {
+ error(getPos(), "Bad DCT precision %d", prec);
+ return gFalse;
+@@ -2945,6 +2973,10 @@ GBool DCTStream::readScanInfo() {
+
+ length = read16() - 2;
+ scanInfo.numComps = str->getChar();
++ if (scanInfo.numComps <= 0 || scanInfo.numComps > 4) {
++ error(getPos(), "Bad number of components in DCT stream");
++ return gFalse;
++ }
+ --length;
+ if (length != 2 * scanInfo.numComps + 3) {
+ error(getPos(), "Bad DCT scan info block");
+@@ -3255,6 +3287,10 @@ FlateStream::FlateStream(Stream *strA, i
+ FilterStream(strA) {
+ if (predictor != 1) {
+ pred = new StreamPredictor(this, predictor, columns, colors, bits);
++ if (!pred->isOk()) {
++ delete pred;
++ pred = NULL;
++ }
+ } else {
+ pred = NULL;
+ }
+--- tetex-bin-3.0/libs/xpdf/xpdf/Stream.h
++++ tetex-bin-3.0.new/libs/xpdf/xpdf/Stream.h
+@@ -233,6 +233,8 @@ public:
+
+ ~StreamPredictor();
+
++ GBool isOk() { return ok; }
++
+ int lookChar();
+ int getChar();
+
+@@ -250,6 +252,7 @@ private:
+ int rowBytes; // bytes per line
+ Guchar *predLine; // line buffer
+ int predIdx; // current index in predLine
++ GBool ok;
+ };
+
+ //------------------------------------------------------------------------
Attachment:
signature.asc
Description: Digital signature