From dca380ffe0571be4023b11b06b8aecad9934bb06 Mon Sep 17 00:00:00 2001 From: Nicolas Pena Date: Fri, 1 Dec 2017 21:40:23 +0000 Subject: Check for success of decodes to avoid infinite loops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: 790693 Change-Id: I9b1d87e024229d8b01f55ec554e2cc544db6ac06 Reviewed-on: https://pdfium-review.googlesource.com/20230 Reviewed-by: Henrique Nakashima 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, 44 insertions(+), 26 deletions(-) (limited to 'core/fxcodec') diff --git a/core/fxcodec/jbig2/JBig2_ArithIntDecoder.cpp b/core/fxcodec/jbig2/JBig2_ArithIntDecoder.cpp index 7ed7702964..8ef1e0dc45 100644 --- a/core/fxcodec/jbig2/JBig2_ArithIntDecoder.cpp +++ b/core/fxcodec/jbig2/JBig2_ArithIntDecoder.cpp @@ -53,6 +53,9 @@ 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); @@ -61,6 +64,9 @@ 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) @@ -92,13 +98,17 @@ CJBig2_ArithIaidDecoder::CJBig2_ArithIaidDecoder(unsigned char SBSYMCODELENA) CJBig2_ArithIaidDecoder::~CJBig2_ArithIaidDecoder() {} -void CJBig2_ArithIaidDecoder::decode(CJBig2_ArithDecoder* pArithDecoder, +bool 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 fd9fa89f19..2de42a09ee 100644 --- a/core/fxcodec/jbig2/JBig2_ArithIntDecoder.h +++ b/core/fxcodec/jbig2/JBig2_ArithIntDecoder.h @@ -17,8 +17,7 @@ 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. + // Returns true on success, and false when an OOB condition occurs. bool decode(CJBig2_ArithDecoder* pArithDecoder, int* nResult); private: @@ -30,7 +29,7 @@ class CJBig2_ArithIaidDecoder { explicit CJBig2_ArithIaidDecoder(unsigned char SBSYMCODELENA); ~CJBig2_ArithIaidDecoder(); - void decode(CJBig2_ArithDecoder* pArithDecoder, uint32_t* nResult); + bool 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 45aa22e655..43768bd823 100644 --- a/core/fxcodec/jbig2/JBig2_SddProc.cpp +++ b/core/fxcodec/jbig2/JBig2_SddProc.cpp @@ -68,7 +68,9 @@ std::unique_ptr CJBig2_SDDProc::decode_Arith( NSYMSDECODED = 0; while (NSYMSDECODED < SDNUMNEWSYMS) { std::unique_ptr BS; - IADH->decode(pArithDecoder, &HCDH); + if (!IADH->decode(pArithDecoder, &HCDH)) + return nullptr; + HCHEIGHT = HCHEIGHT + HCDH; if ((int)HCHEIGHT < 0 || (int)HCHEIGHT > JBIG2_MAX_IMAGE_SIZE) return nullptr; @@ -113,7 +115,8 @@ std::unique_ptr CJBig2_SDDProc::decode_Arith( if (!BS) return nullptr; } else { - IAAI->decode(pArithDecoder, (int*)&REFAGGNINST); + if (!IAAI->decode(pArithDecoder, reinterpret_cast(&REFAGGNINST))) + return nullptr; if (REFAGGNINST > 1) { auto pDecoder = pdfium::MakeUnique(); pDecoder->SBHUFF = SDHUFF; @@ -186,12 +189,11 @@ std::unique_ptr CJBig2_SDDProc::decode_Arith( } else if (REFAGGNINST == 1) { SBNUMSYMS = SDNUMINSYMS + NSYMSDECODED; uint32_t IDI; - IAID->decode(pArithDecoder, &IDI); - IARDX->decode(pArithDecoder, &RDXI); - IARDY->decode(pArithDecoder, &RDYI); - if (IDI >= SBNUMSYMS) + if (!IAID->decode(pArithDecoder, &IDI) || + !IARDX->decode(pArithDecoder, &RDXI) || + !IARDY->decode(pArithDecoder, &RDYI) || IDI >= SBNUMSYMS) { return nullptr; - + } SBSYMS.resize(SBNUMSYMS); std::copy(SDINSYMS, SDINSYMS + SDNUMINSYMS, SBSYMS.begin()); for (size_t i = 0; i < NSYMSDECODED; ++i) @@ -225,10 +227,10 @@ std::unique_ptr CJBig2_SDDProc::decode_Arith( EXFLAGS.resize(SDNUMINSYMS + SDNUMNEWSYMS); num_ex_syms = 0; while (EXINDEX < SDNUMINSYMS + SDNUMNEWSYMS) { - IAEX->decode(pArithDecoder, (int*)&EXRUNLENGTH); - if (EXINDEX + EXRUNLENGTH > SDNUMINSYMS + SDNUMNEWSYMS) + if (!IAEX->decode(pArithDecoder, (int*)&EXRUNLENGTH) || + 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 332e5868ac..78eb78012a 100644 --- a/core/fxcodec/jbig2/JBig2_TrdProc.cpp +++ b/core/fxcodec/jbig2/JBig2_TrdProc.cpp @@ -281,7 +281,9 @@ std::unique_ptr CJBig2_TRDProc::decode_Arith( for (;;) { if (bFirst) { int32_t DFS; - pIAFS->decode(pArithDecoder, &DFS); + if (!pIAFS->decode(pArithDecoder, &DFS)) + return nullptr; + FIRSTS += DFS; CURS = FIRSTS; bFirst = false; @@ -297,8 +299,10 @@ std::unique_ptr CJBig2_TRDProc::decode_Arith( break; int CURT = 0; - if (SBSTRIPS != 1) - pIAIT->decode(pArithDecoder, &CURT); + if (SBSTRIPS != 1) { + if (!pIAIT->decode(pArithDecoder, &CURT)) + return nullptr; + } FX_SAFE_INT32 SAFE_TI = STRIPT + CURT; if (!SAFE_TI.IsValid()) @@ -306,15 +310,16 @@ std::unique_ptr CJBig2_TRDProc::decode_Arith( int32_t TI = SAFE_TI.ValueOrDie(); uint32_t IDI; - pIAID->decode(pArithDecoder, &IDI); - if (IDI >= SBNUMSYMS) + if (!pIAID->decode(pArithDecoder, &IDI) || IDI >= SBNUMSYMS) return nullptr; int RI; - if (SBREFINE == 0) + if (SBREFINE == 0) { RI = 0; - else - pIARI->decode(pArithDecoder, &RI); + } else { + if (!pIARI->decode(pArithDecoder, &RI)) + return nullptr; + } MaybeOwned pIBI; if (RI == 0) { @@ -324,10 +329,12 @@ std::unique_ptr CJBig2_TRDProc::decode_Arith( int32_t RDHI; int32_t RDXI; int32_t RDYI; - pIARDW->decode(pArithDecoder, &RDWI); - pIARDH->decode(pArithDecoder, &RDHI); - pIARDX->decode(pArithDecoder, &RDXI); - pIARDY->decode(pArithDecoder, &RDYI); + if (!pIARDW->decode(pArithDecoder, &RDWI) || + !pIARDH->decode(pArithDecoder, &RDHI) || + !pIARDX->decode(pArithDecoder, &RDXI) || + !pIARDY->decode(pArithDecoder, &RDYI)) { + return nullptr; + } CJBig2_Image* IBOI = SBSYMS[IDI]; if (!IBOI) return nullptr; -- cgit v1.2.3