From e0bbe4aac90e56950f28e322c3d6192c35af5ea0 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Fri, 23 Jan 2015 15:05:43 -0800 Subject: Fix null crash in CheckTrailer. We are making checks in the incorrect order. Also adds two test cases, one for the this crash, and another for the original issue that motivated the patch. Original Patch by Bo at https://codereview.chromium.org/866003003/ BUG=450871 R=bo_xu@foxitsoftware.com Review URL: https://codereview.chromium.org/872563002 --- BUILD.gn | 1 + .../src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp | 6 ++-- fpdfsdk/src/fpdf_dataavail_unittest.cpp | 23 +++++++++++++ pdfium.gyp | 1 + testing/embedder_test.h | 2 +- testing/resources/trailer_as_hexstring.in | 29 +++++++++++++++++ testing/resources/trailer_as_hexstring.pdf | 35 ++++++++++++++++++++ testing/resources/trailer_unterminated.in | 31 ++++++++++++++++++ testing/resources/trailer_unterminated.pdf | 38 ++++++++++++++++++++++ 9 files changed, 162 insertions(+), 4 deletions(-) create mode 100644 fpdfsdk/src/fpdf_dataavail_unittest.cpp create mode 100644 testing/resources/trailer_as_hexstring.in create mode 100644 testing/resources/trailer_as_hexstring.pdf create mode 100644 testing/resources/trailer_unterminated.in create mode 100644 testing/resources/trailer_unterminated.pdf diff --git a/BUILD.gn b/BUILD.gn index 4f97e34752..0822a5ff07 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -813,6 +813,7 @@ test("pdfium_unittests") { test("pdfium_embeddertests") { sources = [ + "fpdfsdk/src/fpdf_dataavail_unittest.cpp", "fpdfsdk/src/fpdfdoc_embeddertest.cpp", "fpdfsdk/src/fpdfview_embeddertest.cpp", "testing/embedder_test.cpp", diff --git a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp index f6253757b6..5dfcc82787 100644 --- a/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp +++ b/core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp @@ -4026,14 +4026,14 @@ FX_BOOL CPDF_DataAvail::CheckTrailer(IFX_DownloadHints* pHints) CFX_SmartPointer file(FX_CreateMemoryStream(pBuf, (size_t)iSize, FALSE)); m_syntaxParser.InitParser((IFX_FileStream*)file, 0); CPDF_Object *pTrailer = m_syntaxParser.GetObject(NULL, 0, 0, 0); - if (pTrailer->GetType() != PDFOBJ_DICTIONARY) { - return FALSE; - } if (!pTrailer) { m_Pos += m_syntaxParser.SavePos(); pHints->AddSegment(m_Pos, iTrailerSize); return FALSE; } + if (pTrailer->GetType() != PDFOBJ_DICTIONARY) { + return FALSE; + } CPDF_Dictionary *pTrailerDict = pTrailer->GetDict(); if (pTrailerDict) { CPDF_Object *pEncrypt = pTrailerDict->GetElement("Encrypt"); diff --git a/fpdfsdk/src/fpdf_dataavail_unittest.cpp b/fpdfsdk/src/fpdf_dataavail_unittest.cpp new file mode 100644 index 0000000000..6081fa52ac --- /dev/null +++ b/fpdfsdk/src/fpdf_dataavail_unittest.cpp @@ -0,0 +1,23 @@ +// Copyright 2015 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "../../testing/embedder_test.h" +#include "../../fpdfsdk/include/fpdfview.h" +#include "../../fpdfsdk/include/fpdfdoc.h" +#include "testing/gtest/include/gtest/gtest.h" + +class FPDFDataAvailEmbeddertest : public EmbedderTest { +}; + +TEST_F(FPDFDataAvailEmbeddertest, TrailerUnterminated) { + // Document must open without crashing but is too malformed to be available. + EXPECT_TRUE(OpenDocument("testing/resources/trailer_unterminated.pdf")); + EXPECT_FALSE(FPDFAvail_IsDocAvail(avail_, &hints_)); +} + +TEST_F(FPDFDataAvailEmbeddertest, TrailerAsHexstring) { + // Document must open without crashing but is too malformed to be available. + EXPECT_TRUE(OpenDocument("testing/resources/trailer_as_hexstring.pdf")); + EXPECT_FALSE(FPDFAvail_IsDocAvail(avail_, &hints_)); +} diff --git a/pdfium.gyp b/pdfium.gyp index d287181dfc..e9c958c703 100644 --- a/pdfium.gyp +++ b/pdfium.gyp @@ -838,6 +838,7 @@ '<(DEPTH)' ], 'sources': [ + 'fpdfsdk/src/fpdf_dataavail_unittest.cpp', 'fpdfsdk/src/fpdfdoc_embeddertest.cpp', 'fpdfsdk/src/fpdfview_embeddertest.cpp', 'testing/embedder_test.cpp', diff --git a/testing/embedder_test.h b/testing/embedder_test.h index 48ea415e19..3eb3be606f 100644 --- a/testing/embedder_test.h +++ b/testing/embedder_test.h @@ -65,7 +65,7 @@ class EmbedderTest : public ::testing::Test { // is prohibited after this call is made. virtual void UnloadPage(FPDF_PAGE page, FPDF_FORMHANDLE form); - private: + protected: FPDF_DOCUMENT document_; FPDF_AVAIL avail_; FX_DOWNLOADHINTS hints_; diff --git a/testing/resources/trailer_as_hexstring.in b/testing/resources/trailer_as_hexstring.in new file mode 100644 index 0000000000..ec2368fab4 --- /dev/null +++ b/testing/resources/trailer_as_hexstring.in @@ -0,0 +1,29 @@ +{{header}} +{{object 1 0}} << + /Type /Catalog + /Pages 2 0 R + /Names << + /Dests 10 0 R + >> + /Dests 14 0 R +>> +endobj +{{object 2 0}} << + /Type /Pages + /Count 1 + /Kids [ + 3 0 R + ] +>> +endobj +{{object 3 0}} << + /Type /Page + /Parent 2 0 R + /MediaBox [0 0 612 792] +>> +endobj +{{xref}} +% trailer erroneously contains a hex string, not a dictionary. +trailer <0000deadbabe0000> +{{startxref}} +%%EOF diff --git a/testing/resources/trailer_as_hexstring.pdf b/testing/resources/trailer_as_hexstring.pdf new file mode 100644 index 0000000000..5b75a53afa --- /dev/null +++ b/testing/resources/trailer_as_hexstring.pdf @@ -0,0 +1,35 @@ +%PDF-1.7 +% ò¤ô +1 0 obj << + /Type /Catalog + /Pages 2 0 R + /Names << + /Dests 10 0 R + >> + /Dests 14 0 R +>> +endobj +2 0 obj << + /Type /Pages + /Count 1 + /Kids [ + 3 0 R + ] +>> +endobj +3 0 obj << + /Type /Page + /Parent 2 0 R + /MediaBox [0 0 612 792] +>> +endobj +xref +0 4 +0000000000 65536 f +0000000015 00000 n +0000000119 00000 n +0000000190 00000 n +trailer <0000deadbabe0000> +startxref +267 +%%EOF diff --git a/testing/resources/trailer_unterminated.in b/testing/resources/trailer_unterminated.in new file mode 100644 index 0000000000..c0c74b749c --- /dev/null +++ b/testing/resources/trailer_unterminated.in @@ -0,0 +1,31 @@ +{{header}} +{{object 1 0}} << + /Type /Catalog + /Pages 2 0 R + /Names << + /Dests 10 0 R + >> + /Dests 14 0 R +>> +endobj +{{object 2 0}} << + /Type /Pages + /Count 1 + /Kids [ + 3 0 R + ] +>> +endobj +{{object 3 0}} << + /Type /Page + /Parent 2 0 R + /MediaBox [0 0 612 792] +>> +endobj +{{xref}} +% closing angle-brackets not present for trailer dictionary. +trailer << + /Size 6 + /Root 1 0 R +{{startxref}} +%%EOF diff --git a/testing/resources/trailer_unterminated.pdf b/testing/resources/trailer_unterminated.pdf new file mode 100644 index 0000000000..b01ec4b67d --- /dev/null +++ b/testing/resources/trailer_unterminated.pdf @@ -0,0 +1,38 @@ +%PDF-1.7 +% ò¤ô +1 0 obj << + /Type /Catalog + /Pages 2 0 R + /Names << + /Dests 10 0 R + >> + /Dests 14 0 R +>> +endobj +2 0 obj << + /Type /Pages + /Count 1 + /Kids [ + 3 0 R + ] +>> +endobj +3 0 obj << + /Type /Page + /Parent 2 0 R + /MediaBox [0 0 612 792] +>> +endobj +xref +0 4 +0000000000 65536 f +0000000015 00000 n +0000000119 00000 n +0000000190 00000 n +% closing angle-brackets not present for trailer dictionary. +trailer << + /Size 6 + /Root 1 0 R +startxref +267 +%%EOF -- cgit v1.2.3