Fix null crash in CheckTrailer.
authorTom Sepez <tsepez@chromium.org>
Fri, 23 Jan 2015 23:05:43 +0000 (15:05 -0800)
committerTom Sepez <tsepez@chromium.org>
Fri, 23 Jan 2015 23:05:43 +0000 (15:05 -0800)
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
core/src/fpdfapi/fpdf_parser/fpdf_parser_parser.cpp
fpdfsdk/src/fpdf_dataavail_unittest.cpp [new file with mode: 0644]
pdfium.gyp
testing/embedder_test.h
testing/resources/trailer_as_hexstring.in [new file with mode: 0644]
testing/resources/trailer_as_hexstring.pdf [new file with mode: 0644]
testing/resources/trailer_unterminated.in [new file with mode: 0644]
testing/resources/trailer_unterminated.pdf [new file with mode: 0644]

index 4f97e34..0822a5f 100644 (file)
--- 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",
index f625375..5dfcc82 100644 (file)
@@ -4026,14 +4026,14 @@ FX_BOOL CPDF_DataAvail::CheckTrailer(IFX_DownloadHints* pHints)
         CFX_SmartPointer<IFX_FileStream> 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 (file)
index 0000000..6081fa5
--- /dev/null
@@ -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_));
+}
index d287181..e9c958c 100644 (file)
         '<(DEPTH)'
       ],
       'sources': [
+        'fpdfsdk/src/fpdf_dataavail_unittest.cpp',
         'fpdfsdk/src/fpdfdoc_embeddertest.cpp',
         'fpdfsdk/src/fpdfview_embeddertest.cpp',
         'testing/embedder_test.cpp',
index 48ea415..3eb3be6 100644 (file)
@@ -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 (file)
index 0000000..ec2368f
--- /dev/null
@@ -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 (file)
index 0000000..5b75a53
--- /dev/null
@@ -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 (file)
index 0000000..c0c74b7
--- /dev/null
@@ -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 (file)
index 0000000..b01ec4b
--- /dev/null
@@ -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