Verify number of ex flags matches number of ex items.

core/fxcodec/jbig2/JBig2_SddProc.cpp

Index: core/fxcodec/jbig2/JBig2_SddProc.cpp
diff --git a/core/fxcodec/jbig2/JBig2_SddProc.cpp b/core/fxcodec/jbig2/JBig2_SddProc.cpp
index 1a7e96d26b2262ca2d7636a457bcc478d05ae51b..c1904176eb4afcc91bbff8a31bf97b2bb9422848 100644
--- a/core/fxcodec/jbig2/JBig2_SddProc.cpp
+++ b/core/fxcodec/jbig2/JBig2_SddProc.cpp
@@ -249,6 +249,11 @@ CJBig2_SymbolDict* CJBig2_SDDProc::decode_Arith(
     EXINDEX = EXINDEX + EXRUNLENGTH;
     CUREXFLAG = !CUREXFLAG;
   }
+  if (EXINDEX > SDNUMEXSYMS) {

[Tom Sepez, 2016/10/12 16:27:43]

This is fine, but generally when writing this "C-Style" error handling, I like
to null out my pointers out top, then always delete them at |failed:|.  Then you
don't have to remember to do this before each |goto failed|

[kcwu, 2016/10/12 16:41:47]

Since SDNUMEXSYMS is always zero, this check means rejecting EXINDEX > 0. I
don't know what these variables are but I feel this seems too aggressive.

I'd suggest do more tests if possible before commit.

[kcwu, 2016/10/12 23:49:58]

So, please ignore my previous stupid comment.

+    FX_Free(EXFLAGS);
+    goto failed;
+  }
+
   pDict.reset(new CJBig2_SymbolDict);
   I = J = 0;
   for (I = 0; I < SDNUMINSYMS + SDNUMNEWSYMS; I++) {
@@ -572,6 +577,11 @@ CJBig2_SymbolDict* CJBig2_SDDProc::decode_Huffman(
     EXINDEX = EXINDEX + EXRUNLENGTH;
     CUREXFLAG = !CUREXFLAG;
   }
+  if (EXINDEX > SDNUMEXSYMS) {

[kcwu, 2016/10/12 23:49:58]

As your commit message said, it will leak if the number of EXFLAGS[]==true is
more than SDNUMEXSYMS. But why do you check EXINDEX > SDNUMEXSYMS here?

EXINDEX includes both 0 and 1 since CUREXFLAG is flipped for each run.

[dsinclair, 2016/10/13 14:12:17]

Ack, good point. Fixed.

+    FX_Free(EXFLAGS);
+    goto failed;
+  }
+
   I = J = 0;
   for (I = 0; I < SDNUMINSYMS + SDNUMNEWSYMS; I++) {
     if (EXFLAGS[I] && J < SDNUMEXSYMS) {