From b36c7e1f84ea7402b7576d2a03a219d469735434 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Pe=C3=B1a=20Moreno?= Date: Mon, 15 Jan 2018 21:47:45 +0000 Subject: Revert "Check for success of decodes to avoid infinite loops" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit dca380ffe0571be4023b11b06b8aecad9934bb06. Reason for revert: Causes missing text in a user's PDF Original change's description: > Check for success of decodes to avoid infinite loops > > Bug: 790693 > Change-Id: I9b1d87e024229d8b01f55ec554e2cc544db6ac06 > Reviewed-on: https://pdfium-review.googlesource.com/20230 > Reviewed-by: Henrique Nakashima > Commit-Queue: Nicolás Peña Moreno TBR=npm@chromium.org,hnakashima@chromium.org,rharrison@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 790693 Change-Id: I886b14e120c34da757a96f8a1f9c6a081d8326b6 Reviewed-on: https://pdfium-review.googlesource.com/22950 Reviewed-by: Nicolás Peña Moreno Commit-Queue: Nicolás Peña Moreno --- core/fxcodec/jbig2/JBig2_ArithIntDecoder.cpp | 12 +---------- core/fxcodec/jbig2/JBig2_ArithIntDecoder.h | 5 +++-- core/fxcodec/jbig2/JBig2_SddProc.cpp | 22 +++++++++----------- core/fxcodec/jbig2/JBig2_TrdProc.cpp | 31 +++++++++++----------------- 4 files changed, 26 insertions(+), 44 deletions(-) diff --git a/core/fxcodec/jbig2/JBig2_ArithIntDecoder.cpp b/core/fxcodec/jbig2/JBig2_ArithIntDecoder.cpp index 8ef1e0dc45..7ed7702964 100644 --- a/core/fxcodec/jbig2/JBig2_ArithIntDecoder.cpp +++ b/core/fxcodec/jbig2/JBig2_ArithIntDecoder.cpp @@ -53,9 +53,6 @@ bool CJBig2_ArithIntDecoder::decode(CJBig2_ArithDecoder* pArithDecoder, // Decoding Procedure" on page 113 of the JBIG2 specification (ISO/IEC FCD // 14492). int PREV = 1; - if (pArithDecoder->IsComplete()) - return false; - const int S = pArithDecoder->DECODE(&m_IAx[PREV]); PREV = ShiftOr(PREV, S); @@ -64,9 +61,6 @@ bool CJBig2_ArithIntDecoder::decode(CJBig2_ArithDecoder* pArithDecoder, int nTemp = 0; for (int i = 0; i < g_ArithIntDecodeData[nDecodeDataIndex].nNeedBits; ++i) { - if (pArithDecoder->IsComplete()) - return false; - int D = pArithDecoder->DECODE(&m_IAx[PREV]); PREV = ShiftOr(PREV, D); if (PREV >= 256) @@ -98,17 +92,13 @@ CJBig2_ArithIaidDecoder::CJBig2_ArithIaidDecoder(unsigned char SBSYMCODELENA) CJBig2_ArithIaidDecoder::~CJBig2_ArithIaidDecoder() {} -bool CJBig2_ArithIaidDecoder::decode(CJBig2_ArithDecoder* pArithDecoder, +void CJBig2_ArithIaidDecoder::decode(CJBig2_ArithDecoder* pArithDecoder, uint32_t* nResult) { int PREV = 1; for (unsigned char i = 0; i < SBSYMCODELEN; ++i) { JBig2ArithCtx* pCX = &m_IAID[PREV]; - if (pArithDecoder->IsComplete()) - return false; - int D = pArithDecoder->DECODE(pCX); PREV = ShiftOr(PREV, D); } *nResult = PREV - (1 << SBSYMCODELEN); - return true; } diff --git a/core/fxcodec/jbig2/JBig2_ArithIntDecoder.h b/core/fxcodec/jbig2/JBig2_ArithIntDecoder.h index 2de42a09ee..fd9fa89f19 100644 --- a/core/fxcodec/jbig2/JBig2_ArithIntDecoder.h +++ b/core/fxcodec/jbig2/JBig2_ArithIntDecoder.h @@ -17,7 +17,8 @@ class CJBig2_ArithIntDecoder { CJBig2_ArithIntDecoder(); ~CJBig2_ArithIntDecoder(); - // Returns true on success, and false when an OOB condition occurs. + // 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: @@ -29,7 +30,7 @@ class CJBig2_ArithIaidDecoder { explicit CJBig2_ArithIaidDecoder(unsigned char SBSYMCODELENA); ~CJBig2_ArithIaidDecoder(); - bool decode(CJBig2_ArithDecoder* pArithDecoder, uint32_t* nResult); + void decode(CJBig2_ArithDecoder* pArithDecoder, uint32_t* nResult); private: std::vector m_IAID; diff --git a/core/fxcodec/jbig2/JBig2_SddProc.cpp b/core/fxcodec/jbig2/JBig2_SddProc.cpp index ee0c0f37f3..a446dbc6c0 100644 --- a/core/fxcodec/jbig2/JBig2_SddProc.cpp +++ b/core/fxcodec/jbig2/JBig2_SddProc.cpp @@ -68,9 +68,7 @@ std::unique_ptr CJBig2_SDDProc::decode_Arith( NSYMSDECODED = 0; while (NSYMSDECODED < SDNUMNEWSYMS) { std::unique_ptr BS; - if (!IADH->decode(pArithDecoder, &HCDH)) - return nullptr; - + IADH->decode(pArithDecoder, &HCDH); HCHEIGHT = HCHEIGHT + HCDH; if ((int)HCHEIGHT < 0 || (int)HCHEIGHT > JBIG2_MAX_IMAGE_SIZE) return nullptr; @@ -115,8 +113,7 @@ std::unique_ptr CJBig2_SDDProc::decode_Arith( if (!BS) return nullptr; } else { - if (!IAAI->decode(pArithDecoder, reinterpret_cast(&REFAGGNINST))) - return nullptr; + IAAI->decode(pArithDecoder, (int*)&REFAGGNINST); if (REFAGGNINST > 1) { auto pDecoder = pdfium::MakeUnique(); pDecoder->SBHUFF = SDHUFF; @@ -189,11 +186,12 @@ std::unique_ptr CJBig2_SDDProc::decode_Arith( } else if (REFAGGNINST == 1) { SBNUMSYMS = SDNUMINSYMS + NSYMSDECODED; uint32_t IDI; - if (!IAID->decode(pArithDecoder, &IDI) || - !IARDX->decode(pArithDecoder, &RDXI) || - !IARDY->decode(pArithDecoder, &RDYI) || IDI >= SBNUMSYMS) { + IAID->decode(pArithDecoder, &IDI); + IARDX->decode(pArithDecoder, &RDXI); + IARDY->decode(pArithDecoder, &RDYI); + if (IDI >= SBNUMSYMS) return nullptr; - } + SBSYMS.resize(SBNUMSYMS); std::copy(SDINSYMS, SDINSYMS + SDNUMINSYMS, SBSYMS.begin()); for (size_t i = 0; i < NSYMSDECODED; ++i) @@ -227,10 +225,10 @@ std::unique_ptr CJBig2_SDDProc::decode_Arith( EXFLAGS.resize(SDNUMINSYMS + SDNUMNEWSYMS); num_ex_syms = 0; while (EXINDEX < SDNUMINSYMS + SDNUMNEWSYMS) { - if (!IAEX->decode(pArithDecoder, (int*)&EXRUNLENGTH) || - EXINDEX + EXRUNLENGTH > SDNUMINSYMS + SDNUMNEWSYMS) { + IAEX->decode(pArithDecoder, (int*)&EXRUNLENGTH); + if (EXINDEX + EXRUNLENGTH > SDNUMINSYMS + SDNUMNEWSYMS) return nullptr; - } + if (EXRUNLENGTH != 0) { for (I = EXINDEX; I < EXINDEX + EXRUNLENGTH; I++) { if (CUREXFLAG) diff --git a/core/fxcodec/jbig2/JBig2_TrdProc.cpp b/core/fxcodec/jbig2/JBig2_TrdProc.cpp index 78eb78012a..332e5868ac 100644 --- a/core/fxcodec/jbig2/JBig2_TrdProc.cpp +++ b/core/fxcodec/jbig2/JBig2_TrdProc.cpp @@ -281,9 +281,7 @@ std::unique_ptr CJBig2_TRDProc::decode_Arith( for (;;) { if (bFirst) { int32_t DFS; - if (!pIAFS->decode(pArithDecoder, &DFS)) - return nullptr; - + pIAFS->decode(pArithDecoder, &DFS); FIRSTS += DFS; CURS = FIRSTS; bFirst = false; @@ -299,10 +297,8 @@ std::unique_ptr CJBig2_TRDProc::decode_Arith( break; int CURT = 0; - if (SBSTRIPS != 1) { - if (!pIAIT->decode(pArithDecoder, &CURT)) - return nullptr; - } + if (SBSTRIPS != 1) + pIAIT->decode(pArithDecoder, &CURT); FX_SAFE_INT32 SAFE_TI = STRIPT + CURT; if (!SAFE_TI.IsValid()) @@ -310,16 +306,15 @@ std::unique_ptr CJBig2_TRDProc::decode_Arith( int32_t TI = SAFE_TI.ValueOrDie(); uint32_t IDI; - if (!pIAID->decode(pArithDecoder, &IDI) || IDI >= SBNUMSYMS) + pIAID->decode(pArithDecoder, &IDI); + if (IDI >= SBNUMSYMS) return nullptr; int RI; - if (SBREFINE == 0) { + if (SBREFINE == 0) RI = 0; - } else { - if (!pIARI->decode(pArithDecoder, &RI)) - return nullptr; - } + else + pIARI->decode(pArithDecoder, &RI); MaybeOwned pIBI; if (RI == 0) { @@ -329,12 +324,10 @@ std::unique_ptr CJBig2_TRDProc::decode_Arith( int32_t RDHI; int32_t RDXI; int32_t RDYI; - if (!pIARDW->decode(pArithDecoder, &RDWI) || - !pIARDH->decode(pArithDecoder, &RDHI) || - !pIARDX->decode(pArithDecoder, &RDXI) || - !pIARDY->decode(pArithDecoder, &RDYI)) { - return nullptr; - } + pIARDW->decode(pArithDecoder, &RDWI); + pIARDH->decode(pArithDecoder, &RDHI); + pIARDX->decode(pArithDecoder, &RDXI); + pIARDY->decode(pArithDecoder, &RDYI); CJBig2_Image* IBOI = SBSYMS[IDI]; if (!IBOI) return nullptr; -- cgit v1.2.3