From 0233ff952372e9a15edf92beccba463d74c46c33 Mon Sep 17 00:00:00 2001
From: Miklos Vajna <vmiklos@collabora.co.uk>
Date: Thu, 1 Dec 2016 09:42:45 +0100
Subject: xmlsecurity PDF verify: tolerate missing %%EOF in incremental updates

This is broken, but work it around to avoid an infinite loop.

Change-Id: I132a3c19cfe53e6166bfc1a881d1bfa5071f85d4
Reviewed-on: https://gerrit.libreoffice.org/31471
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Miklos Vajna <vmiklos@collabora.co.uk>
---
 xmlsecurity/qa/unit/pdfsigning/data/no-eof.pdf | Bin 0 -> 174879 bytes
 xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx  |  20 ++++++++++++++++++++
 xmlsecurity/source/pdfio/pdfdocument.cxx       |  12 ++++++++----
 3 files changed, 28 insertions(+), 4 deletions(-)
 create mode 100644 xmlsecurity/qa/unit/pdfsigning/data/no-eof.pdf

diff --git a/xmlsecurity/qa/unit/pdfsigning/data/no-eof.pdf b/xmlsecurity/qa/unit/pdfsigning/data/no-eof.pdf
new file mode 100644
index 000000000000..9ae7e23bb019
Binary files /dev/null and b/xmlsecurity/qa/unit/pdfsigning/data/no-eof.pdf differ
diff --git a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
index 5b88c71d90d2..f22a7c6410bd 100644
--- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
+++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
@@ -68,6 +68,8 @@ public:
     void testSigningCertificateAttribute();
     /// Test that we accept files which are supposed to be good.
     void testGood();
+    /// Test that we don't crash / loop while tokenizing these files.
+    void testTokenize();
 
     CPPUNIT_TEST_SUITE(PDFSigningTest);
     CPPUNIT_TEST(testPDFAdd);
@@ -81,6 +83,7 @@ public:
     CPPUNIT_TEST(testPDFPAdESGood);
     CPPUNIT_TEST(testSigningCertificateAttribute);
     CPPUNIT_TEST(testGood);
+    CPPUNIT_TEST(testTokenize);
     CPPUNIT_TEST_SUITE_END();
 };
 
@@ -366,6 +369,23 @@ void PDFSigningTest::testGood()
 #endif
 }
 
+void PDFSigningTest::testTokenize()
+{
+    const std::initializer_list<OUStringLiteral> aNames =
+    {
+        // We looped on this broken input.
+        OUStringLiteral("no-eof.pdf"),
+    };
+
+    for (const auto& rName : aNames)
+    {
+        SvFileStream aStream(m_directories.getURLFromSrc(DATA_DIRECTORY) + rName, StreamMode::READ);
+        xmlsecurity::pdfio::PDFDocument aDocument;
+        // Just make sure the tokenizer finishes without an error, don't look at the signature.
+        CPPUNIT_ASSERT(aDocument.Read(aStream));
+    }
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(PDFSigningTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/xmlsecurity/source/pdfio/pdfdocument.cxx b/xmlsecurity/source/pdfio/pdfdocument.cxx
index e3e89a0750ee..86aff1eb3f5a 100644
--- a/xmlsecurity/source/pdfio/pdfdocument.cxx
+++ b/xmlsecurity/source/pdfio/pdfdocument.cxx
@@ -1228,8 +1228,12 @@ bool PDFDocument::Tokenize(SvStream& rStream, TokenizeMode eMode, std::vector< s
                 }
                 else if (aKeyword == "trailer")
                 {
-                    m_pTrailer = new PDFTrailerElement(*this);
-                    rElements.push_back(std::unique_ptr<PDFElement>(m_pTrailer));
+                    auto pTrailer = new PDFTrailerElement(*this);
+                    // When reading till the first EOF token only, remember
+                    // just the first trailer token.
+                    if (eMode != TokenizeMode::EOF_TOKEN || !m_pTrailer)
+                        m_pTrailer = pTrailer;
+                    rElements.push_back(std::unique_ptr<PDFElement>(pTrailer));
                 }
                 else if (aKeyword == "startxref")
                 {
@@ -1680,9 +1684,9 @@ void PDFDocument::ReadXRef(SvStream& rStream)
             return;
         }
 
-        if (aNumberOfEntries.GetValue() <= 0)
+        if (aNumberOfEntries.GetValue() < 0)
         {
-            SAL_WARN("xmlsecurity.pdfio", "PDFDocument::ReadXRef: expected one or more entries");
+            SAL_WARN("xmlsecurity.pdfio", "PDFDocument::ReadXRef: expected zero or more entries");
             return;
         }
 
-- 
cgit