Hi Frank, hi Florian! Frank Küster [2005-12-08 13:17 +0100]: > Martin Pitt <[EMAIL PROTECTED]> 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; + }; + + //------------------------------------------------------------------------
signature.asc
Description: Digital signature