From 182d129bcee8f7731b9bbfde0064295ad3b37271 Mon Sep 17 00:00:00 2001 From: Oliver Chang Date: Thu, 12 Nov 2015 10:43:27 -0800 Subject: Clear decoders after the image decoder in the /Filter array. During decoding, when an image decoder is encountered, any subsequent decoders are ignored, but remain in the array. However, later on CPDF_DIBSource::ValidateDictParam expects the image decoder to be the last in the array, causing issues. A check is also added in CPDF_DIBSource::GetScanline to ensure that the calculated pitch value is <= the (4-aligned) pitch value in the cached bitmap to prevent future issues. Also cleans up some NULL usages. BUG=552046 R=jun_fang@foxitsoftware.com, tsepez@chromium.org Review URL: https://codereview.chromium.org/1406943005 . --- core/include/fpdfapi/fpdf_objects.h | 2 +- core/src/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp | 11 +++++++---- .../fpdf_parser/fpdf_parser_decode_embeddertest.cpp | 10 ++++++++++ core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp | 15 ++++++++++----- core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp | 10 +++++----- testing/resources/bug_552046.in | Bin 0 -> 624 bytes testing/resources/bug_552046.pdf | Bin 0 -> 746 bytes 7 files changed, 33 insertions(+), 15 deletions(-) create mode 100644 testing/resources/bug_552046.in create mode 100644 testing/resources/bug_552046.pdf diff --git a/core/include/fpdfapi/fpdf_objects.h b/core/include/fpdfapi/fpdf_objects.h index 11589e65a2..b754ab2b2b 100644 --- a/core/include/fpdfapi/fpdf_objects.h +++ b/core/include/fpdfapi/fpdf_objects.h @@ -323,7 +323,7 @@ class CPDF_Array : public CPDF_Object { CPDF_Object* pObj, CPDF_IndirectObjects* pObjs = NULL); - void RemoveAt(FX_DWORD index); + void RemoveAt(FX_DWORD index, int nCount = 1); void Add(CPDF_Object* pObj, CPDF_IndirectObjects* pObjs = NULL); diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp index ff0519c9b5..588ab5dff6 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_decode.cpp @@ -363,7 +363,7 @@ FX_BOOL PDF_DataDecode(const uint8_t* src_buf, // Use ToDictionary here because we can push NULL into the ParamList. CPDF_Dictionary* pParam = ToDictionary(static_cast(ParamList[i])); - uint8_t* new_buf = NULL; + uint8_t* new_buf = nullptr; FX_DWORD new_size = (FX_DWORD)-1; int offset = -1; if (decoder == FX_BSTRC("FlateDecode") || decoder == FX_BSTRC("Fl")) { @@ -395,18 +395,21 @@ FX_BOOL PDF_DataDecode(const uint8_t* src_buf, return TRUE; } offset = RunLengthDecode(last_buf, last_size, new_buf, new_size); + } else if (decoder == FX_BSTRC("Crypt")) { + continue; } else { + // If we get here, assume it's an image decoder. if (decoder == FX_BSTRC("DCT")) { decoder = "DCTDecode"; } else if (decoder == FX_BSTRC("CCF")) { decoder = "CCITTFaxDecode"; - } else if (decoder == FX_BSTRC("Crypt")) { - continue; } ImageEncoding = decoder; pImageParms = pParam; dest_buf = (uint8_t*)last_buf; dest_size = last_size; + if (CPDF_Array* pDecoders = pDecoder->AsArray()) + pDecoders->RemoveAt(i + 1, pDecoders->GetCount() - i - 1); return TRUE; } if (last_buf != src_buf) { @@ -420,7 +423,7 @@ FX_BOOL PDF_DataDecode(const uint8_t* src_buf, last_size = new_size; } ImageEncoding = ""; - pImageParms = NULL; + pImageParms = nullptr; dest_buf = last_buf; dest_size = last_size; return TRUE; diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp index 279151a87a..62da88b9ba 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp @@ -95,4 +95,14 @@ TEST_F(FPDFParserDecodeEmbeddertest, FlateDecode) { } } +TEST_F(FPDFParserDecodeEmbeddertest, Bug_552046) { + // Tests specifying multiple image filters for a stream. Should not cause a + // crash when rendered. + EXPECT_TRUE(OpenDocument("bug_552046.pdf")); + FPDF_PAGE page = LoadPage(0); + FPDF_BITMAP bitmap = RenderPage(page); + FPDFBitmap_Destroy(bitmap); + UnloadPage(page); +} + #undef TEST_CASE diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp index 990bf5f0e2..edf80d0618 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_objects.cpp @@ -496,13 +496,18 @@ CPDF_Stream* CPDF_Array::GetStream(FX_DWORD i) const { CPDF_Array* CPDF_Array::GetArray(FX_DWORD i) const { return ToArray(GetElementValue(i)); } -void CPDF_Array::RemoveAt(FX_DWORD i) { - ASSERT(IsArray()); +void CPDF_Array::RemoveAt(FX_DWORD i, int nCount) { if (i >= (FX_DWORD)m_Objects.GetSize()) return; - if (CPDF_Object* p = static_cast(m_Objects.GetAt(i))) - p->Release(); - m_Objects.RemoveAt(i); + + if (nCount <= 0 || nCount > m_Objects.GetSize() - i) + return; + + for (int j = 0; j < nCount; ++j) { + if (CPDF_Object* p = static_cast(m_Objects.GetAt(i + j))) + p->Release(); + } + m_Objects.RemoveAt(i, nCount); } void CPDF_Array::SetAt(FX_DWORD i, CPDF_Object* pObj, diff --git a/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp b/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp index bdd5e9aa9c..a7941b4b2d 100644 --- a/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp +++ b/core/src/fpdfapi/fpdf_render/fpdf_render_loadimage.cpp @@ -1084,14 +1084,14 @@ uint8_t* CPDF_DIBSource::GetBuffer() const { } const uint8_t* CPDF_DIBSource::GetScanline(int line) const { if (m_bpc == 0) { - return NULL; + return nullptr; } FX_SAFE_DWORD src_pitch = CalculatePitch8(m_bpc, m_nComponents, m_Width); if (!src_pitch.IsValid()) - return NULL; + return nullptr; FX_DWORD src_pitch_value = src_pitch.ValueOrDie(); - const uint8_t* pSrcLine = NULL; - if (m_pCachedBitmap) { + const uint8_t* pSrcLine = nullptr; + if (m_pCachedBitmap && src_pitch_value <= m_pCachedBitmap->GetPitch()) { if (line >= m_pCachedBitmap->GetHeight()) { line = m_pCachedBitmap->GetHeight() - 1; } @@ -1103,7 +1103,7 @@ const uint8_t* CPDF_DIBSource::GetScanline(int line) const { pSrcLine = m_pStreamAcc->GetData() + line * src_pitch_value; } } - if (pSrcLine == NULL) { + if (!pSrcLine) { uint8_t* pLineBuf = m_pMaskedLine ? m_pMaskedLine : m_pLineBuf; FXSYS_memset(pLineBuf, 0xFF, m_Pitch); return pLineBuf; diff --git a/testing/resources/bug_552046.in b/testing/resources/bug_552046.in new file mode 100644 index 0000000000..2afc252c75 Binary files /dev/null and b/testing/resources/bug_552046.in differ diff --git a/testing/resources/bug_552046.pdf b/testing/resources/bug_552046.pdf new file mode 100644 index 0000000000..a60432dc95 Binary files /dev/null and b/testing/resources/bug_552046.pdf differ -- cgit v1.2.3