Fix regression in JBIG2 decoding from commit ce37d73.
authorLei Zhang <thestig@chromium.org>
Tue, 6 Oct 2015 06:02:25 +0000 (23:02 -0700)
committerLei Zhang <thestig@chromium.org>
Tue, 6 Oct 2015 06:02:25 +0000 (23:02 -0700)
many callers can tolerate CJBig2_ArithIntDecoder::decode() OOB failure.

BUG=539749, pdfium:209
R=tsepez@chromium.org

Review URL: https://codereview.chromium.org/1384663007 .

core/src/fxcodec/jbig2/JBig2_ArithIntDecoder.h
core/src/fxcodec/jbig2/JBig2_SddProc.cpp
core/src/fxcodec/jbig2/JBig2_TrdProc.cpp

index f31636b..391004b 100644 (file)
@@ -17,6 +17,8 @@ class CJBig2_ArithIntDecoder {
   CJBig2_ArithIntDecoder();
   ~CJBig2_ArithIntDecoder();
 
+  // Returns true on success, and false when an OOB condition occurs. Many
+  // callers can tolerate OOB and do not check the return value.
   bool decode(CJBig2_ArithDecoder* pArithDecoder, int* nResult);
 
  private:
index afce6eb..16f4a90 100644 (file)
@@ -31,7 +31,6 @@ CJBig2_SymbolDict* CJBig2_SDDProc::decode_Arith(
   FX_DWORD EXINDEX;
   FX_BOOL CUREXFLAG;
   FX_DWORD EXRUNLENGTH;
-  int32_t nVal;
   FX_DWORD nTmp;
   FX_DWORD SBNUMSYMS;
   uint8_t SBSYMCODELEN;
@@ -64,9 +63,7 @@ CJBig2_SymbolDict* CJBig2_SDDProc::decode_Arith(
   NSYMSDECODED = 0;
   while (NSYMSDECODED < SDNUMNEWSYMS) {
     BS = nullptr;
-    if (!IADH->decode(pArithDecoder, &HCDH)) {
-      goto failed;
-    }
+    IADH->decode(pArithDecoder, &HCDH);
     HCHEIGHT = HCHEIGHT + HCDH;
     if ((int)HCHEIGHT < 0 || (int)HCHEIGHT > JBIG2_MAX_IMAGE_SIZE) {
       goto failed;
@@ -74,26 +71,23 @@ CJBig2_SymbolDict* CJBig2_SDDProc::decode_Arith(
     SYMWIDTH = 0;
     TOTWIDTH = 0;
     for (;;) {
-      nVal = IADW->decode(pArithDecoder, &DW);
-      if (nVal == JBIG2_OOB) {
+      if (!IADW->decode(pArithDecoder, &DW))
         break;
-      } else if (nVal != 0) {
+
+      if (NSYMSDECODED >= SDNUMNEWSYMS)
         goto failed;
-      } else {
-        if (NSYMSDECODED >= SDNUMNEWSYMS) {
-          goto failed;
-        }
-        SYMWIDTH = SYMWIDTH + DW;
-        if ((int)SYMWIDTH < 0 || (int)SYMWIDTH > JBIG2_MAX_IMAGE_SIZE) {
-          goto failed;
-        } else if (HCHEIGHT == 0 || SYMWIDTH == 0) {
-          TOTWIDTH = TOTWIDTH + SYMWIDTH;
-          SDNEWSYMS[NSYMSDECODED] = nullptr;
-          NSYMSDECODED = NSYMSDECODED + 1;
-          continue;
-        }
+
+      SYMWIDTH = SYMWIDTH + DW;
+      if ((int)SYMWIDTH < 0 || (int)SYMWIDTH > JBIG2_MAX_IMAGE_SIZE)
+        goto failed;
+
+      if (HCHEIGHT == 0 || SYMWIDTH == 0) {
         TOTWIDTH = TOTWIDTH + SYMWIDTH;
+        SDNEWSYMS[NSYMSDECODED] = nullptr;
+        NSYMSDECODED = NSYMSDECODED + 1;
+        continue;
       }
+      TOTWIDTH = TOTWIDTH + SYMWIDTH;
       if (SDREFAGG == 0) {
         nonstd::unique_ptr<CJBig2_GRDProc> pGRD(new CJBig2_GRDProc());
         pGRD->MMR = 0;
@@ -115,9 +109,7 @@ CJBig2_SymbolDict* CJBig2_SDDProc::decode_Arith(
           goto failed;
         }
       } else {
-        if (!IAAI->decode(pArithDecoder, (int*)&REFAGGNINST)) {
-          goto failed;
-        }
+        IAAI->decode(pArithDecoder, (int*)&REFAGGNINST);
         if (REFAGGNINST > 1) {
           nonstd::unique_ptr<CJBig2_TRDProc> pDecoder(new CJBig2_TRDProc());
           pDecoder->SBHUFF = SDHUFF;
@@ -210,10 +202,8 @@ CJBig2_SymbolDict* CJBig2_SDDProc::decode_Arith(
           SBNUMSYMS = SDNUMINSYMS + NSYMSDECODED;
           FX_DWORD IDI;
           IAID->decode(pArithDecoder, &IDI);
-          if (!IARDX->decode(pArithDecoder, &RDXI) ||
-              !IARDY->decode(pArithDecoder, &RDYI)) {
-            goto failed;
-          }
+          IARDX->decode(pArithDecoder, &RDXI);
+          IARDY->decode(pArithDecoder, &RDYI);
           if (IDI >= SBNUMSYMS) {
             goto failed;
           }
@@ -254,10 +244,7 @@ CJBig2_SymbolDict* CJBig2_SDDProc::decode_Arith(
   CUREXFLAG = 0;
   EXFLAGS = FX_Alloc(FX_BOOL, SDNUMINSYMS + SDNUMNEWSYMS);
   while (EXINDEX < SDNUMINSYMS + SDNUMNEWSYMS) {
-    if (!IAEX->decode(pArithDecoder, (int*)&EXRUNLENGTH)) {
-      FX_Free(EXFLAGS);
-      goto failed;
-    }
+    IAEX->decode(pArithDecoder, (int*)&EXRUNLENGTH);
     if (EXINDEX + EXRUNLENGTH > SDNUMINSYMS + SDNUMNEWSYMS) {
       FX_Free(EXFLAGS);
       goto failed;
index f26bdb6..368ec9b 100644 (file)
@@ -226,7 +226,6 @@ CJBig2_Image* CJBig2_TRDProc::decode_Arith(CJBig2_ArithDecoder* pArithDecoder,
   CJBig2_Image* IBOI;
   FX_DWORD WOI, HOI;
   FX_BOOL bFirst;
-  int32_t nRet;
   int32_t bRetained;
   CJBig2_ArithIntDecoder* IADT, *IAFS, *IADS, *IAIT, *IARI, *IARDW, *IARDH,
       *IARDX, *IARDY;
@@ -258,72 +257,55 @@ CJBig2_Image* CJBig2_TRDProc::decode_Arith(CJBig2_ArithDecoder* pArithDecoder,
   }
   nonstd::unique_ptr<CJBig2_Image> SBREG(new CJBig2_Image(SBW, SBH));
   SBREG->fill(SBDEFPIXEL);
-  if (!IADT->decode(pArithDecoder, &STRIPT)) {
-    goto failed;
-  }
+  IADT->decode(pArithDecoder, &STRIPT);
   STRIPT *= SBSTRIPS;
   STRIPT = -STRIPT;
   FIRSTS = 0;
   NINSTANCES = 0;
   while (NINSTANCES < SBNUMINSTANCES) {
-    if (!IADT->decode(pArithDecoder, &DT)) {
-      goto failed;
-    }
+    IADT->decode(pArithDecoder, &DT);
     DT *= SBSTRIPS;
     STRIPT = STRIPT + DT;
     bFirst = TRUE;
     for (;;) {
       if (bFirst) {
-        if (!IAFS->decode(pArithDecoder, &DFS)) {
-          goto failed;
-        }
+        IAFS->decode(pArithDecoder, &DFS);
         FIRSTS = FIRSTS + DFS;
         CURS = FIRSTS;
         bFirst = FALSE;
       } else {
-        nRet = IADS->decode(pArithDecoder, &IDS);
-        if (nRet == JBIG2_OOB) {
+        if (!IADS->decode(pArithDecoder, &IDS))
           break;
-        } else if (nRet != 0) {
-          goto failed;
-        } else {
-          CURS = CURS + IDS + SBDSOFFSET;
-        }
+        CURS = CURS + IDS + SBDSOFFSET;
       }
       if (NINSTANCES >= SBNUMINSTANCES) {
         break;
       }
       int CURT = 0;
-      if (SBSTRIPS != 1) {
-        if (!IAIT->decode(pArithDecoder, &CURT)) {
-          goto failed;
-        }
-      }
+      if (SBSTRIPS != 1)
+        IAIT->decode(pArithDecoder, &CURT);
+
       TI = STRIPT + CURT;
       FX_DWORD IDI;
       IAID->decode(pArithDecoder, &IDI);
-      if (IDI >= SBNUMSYMS) {
+      if (IDI >= SBNUMSYMS)
         goto failed;
-      }
-      if (SBREFINE == 0) {
+
+      if (SBREFINE == 0)
         RI = 0;
-      } else {
-        if (!IARI->decode(pArithDecoder, &RI)) {
-          goto failed;
-        }
-      }
-      if (!SBSYMS[IDI]) {
+      else
+        IARI->decode(pArithDecoder, &RI);
+
+      if (!SBSYMS[IDI])
         goto failed;
-      }
+
       if (RI == 0) {
         IBI = SBSYMS[IDI];
       } else {
-        if (!IARDW->decode(pArithDecoder, &RDWI) ||
-            !IARDH->decode(pArithDecoder, &RDHI) ||
-            !IARDX->decode(pArithDecoder, &RDXI) ||
-            !IARDY->decode(pArithDecoder, &RDYI)) {
-          goto failed;
-        }
+        IARDW->decode(pArithDecoder, &RDWI);
+        IARDH->decode(pArithDecoder, &RDHI);
+        IARDX->decode(pArithDecoder, &RDXI);
+        IARDY->decode(pArithDecoder, &RDYI);
         IBOI = SBSYMS[IDI];
         WOI = IBOI->m_nWidth;
         HOI = IBOI->m_nHeight;