xmlsecurity: detect unsigned incremental update between signatures

(cherry picked from commit 7468d5df5ec79783eae84b62bdc5ecf12f0ca255)

Conflicts:
vcl/source/filter/ipdf/pdfdocument.cxx
xmlsecurity/inc/pdfio/pdfdocument.hxx
xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
xmlsecurity/source/pdfio/pdfdocument.cxx

Change-Id: I269ed858852ee7d1275adf340c8cc1565fc30693
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99510
Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoffice@gmail.com>
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>

6 files changed, 139 insertions, 32 deletions

diff --git a/xmlsecurity/inc/pdfio/pdfdocument.hxx b/xmlsecurity/inc/pdfio/pdfdocument.hxx
index 622bc1ffb678..aaacae9db29f 100644
--- a/xmlsecurity/inc/pdfio/pdfdocument.hxx
+++ b/xmlsecurity/inc/pdfio/pdfdocument.hxx
@@ -26,10 +26,10 @@ namespace pdfio
 
 /**
  * @param rInformation The actual result.
- * @param bLast If this is the last signature in the file, so it covers the whole file physically.
+ * @param rDocument the parsed document to see if the signature is partial.
  * @return If we can determinate a result.
  */
-XMLSECURITY_DLLPUBLIC bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, bool bLast);
+XMLSECURITY_DLLPUBLIC bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, vcl::filter::PDFDocument& rDocument);
 
 } // namespace pdfio
 } // namespace xmlsecurity
diff --git a/xmlsecurity/qa/unit/pdfsigning/data/partial-in-between.pdf b/xmlsecurity/qa/unit/pdfsigning/data/partial-in-between.pdf
new file mode 100644
index 000000000000..211a111cb394
--- /dev/null
+++ b/xmlsecurity/qa/unit/pdfsigning/data/partial-in-between.pdf
diff --git a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
index 2a65ae004e43..54fe2e7912f5 100644
--- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
+++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
@@ -71,6 +71,7 @@ public:
     void testPDFPAdESGood();
     /// Test a valid signature that does not cover the whole file.
     void testPartial();
+    void testPartialInBetween();
     /// Test writing a PAdES signature.
     void testSigningCertificateAttribute();
     /// Test that we accept files which are supposed to be good.
@@ -92,6 +93,7 @@ public:
     CPPUNIT_TEST(testPDF14LOWin);
     CPPUNIT_TEST(testPDFPAdESGood);
     CPPUNIT_TEST(testPartial);
+    CPPUNIT_TEST(testPartialInBetween);
     CPPUNIT_TEST(testSigningCertificateAttribute);
     CPPUNIT_TEST(testGood);
     CPPUNIT_TEST(testTokenize);
@@ -137,8 +139,7 @@ std::vector<SignatureInformation> PDFSigningTest::verify(const OUString& rURL, s
     for (size_t i = 0; i < aSignatures.size(); ++i)
     {
         SignatureInformation aInfo(i);
-        bool bLast = i == aSignatures.size() - 1;
-        CPPUNIT_ASSERT(xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[i], aInfo, bLast));
+        CPPUNIT_ASSERT(xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[i], aInfo, aVerifyDocument));
         aRet.push_back(aInfo);
 
         if (!rExpectedSubFilter.isEmpty())
@@ -267,7 +268,7 @@ void PDFSigningTest::testPDFRemove()
         std::vector<vcl::filter::PDFObjectElement*> aSignatures = aDocument.GetSignatureWidgets();
         CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aSignatures.size());
         SignatureInformation aInfo(0);
-        CPPUNIT_ASSERT(xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[0], aInfo, /*bLast=*/true));
+        CPPUNIT_ASSERT(xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[0], aInfo, aDocument));
     }
 
     // Remove the signature and write out the result as remove.pdf.
@@ -483,6 +484,18 @@ void PDFSigningTest::testUnknownSubFilter()
     CPPUNIT_ASSERT_EQUAL(static_cast<std::size_t>(2), rInformations.size());
 }
 
+void PDFSigningTest::testPartialInBetween()
+{
+    std::vector<SignatureInformation> aInfos
+        = verify(m_directories.getURLFromSrc(DATA_DIRECTORY) + "partial-in-between.pdf", 2,
+                 /*rExpectedSubFilter=*/OString());
+    CPPUNIT_ASSERT(!aInfos.empty());
+    SignatureInformation& rInformation = aInfos[0];
+    // Without the accompanying fix in place, this test would have failed, as unsigned incremental
+    // update between two signatures were not detected.
+    CPPUNIT_ASSERT(rInformation.bPartialDocumentSignature);
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(PDFSigningTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/xmlsecurity/source/helper/pdfsignaturehelper.cxx b/xmlsecurity/source/helper/pdfsignaturehelper.cxx
index 5356705f7b17..5e7684ca24c3 100644
--- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx
+++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx
@@ -52,8 +52,7 @@ bool PDFSignatureHelper::ReadAndVerifySignature(const uno::Reference<io::XInputS
     {
         SignatureInformation aInfo(i);
 
-        bool bLast = i == aSignatures.size() - 1;
-        if (!xmlsecurity::pdfio::ValidateSignature(*pStream, aSignatures[i], aInfo, bLast))
+        if (!xmlsecurity::pdfio::ValidateSignature(*pStream, aSignatures[i], aInfo, aDocument))
             SAL_WARN("xmlsecurity.helper", "failed to determine digest match");
 
         m_aSignatureInfos.push_back(aInfo);
diff --git a/xmlsecurity/source/pdfio/pdfdocument.cxx b/xmlsecurity/source/pdfio/pdfdocument.cxx
index 08437b2f8cae..8d6aeafdd9f3 100644
--- a/xmlsecurity/source/pdfio/pdfdocument.cxx
+++ b/xmlsecurity/source/pdfio/pdfdocument.cxx
@@ -52,12 +52,124 @@
 
 using namespace com::sun::star;
 
+namespace
+{
+/// Turns an array of floats into offset + length pairs.
+bool GetByteRangesFromPDF(vcl::filter::PDFArrayElement& rArray,
+                          std::vector<std::pair<size_t, size_t>>& rByteRanges)
+{
+    size_t nByteRangeOffset = 0;
+    const std::vector<vcl::filter::PDFElement*>& rByteRangeElements = rArray.GetElements();
+    for (size_t i = 0; i < rByteRangeElements.size(); ++i)
+    {
+        auto pNumber = dynamic_cast<vcl::filter::PDFNumberElement*>(rByteRangeElements[i]);
+        if (!pNumber)
+        {
+            SAL_WARN("xmlsecurity.pdfio",
+                     "ValidateSignature: signature offset and length has to be a number");
+            return false;
+        }
+
+        if (i % 2 == 0)
+        {
+            nByteRangeOffset = pNumber->GetValue();
+            continue;
+        }
+        size_t nByteRangeLength = pNumber->GetValue();
+        rByteRanges.emplace_back(nByteRangeOffset, nByteRangeLength);
+    }
+
+    return true;
+}
+
+/// Determines the last position that is covered by a signature.
+bool GetEOFOfSignature(vcl::filter::PDFObjectElement* pSignature, size_t& rEOF)
+{
+    vcl::filter::PDFObjectElement* pValue = pSignature->LookupObject("V");
+    if (!pValue)
+    {
+        return false;
+    }
+
+    auto pByteRange = dynamic_cast<vcl::filter::PDFArrayElement*>(pValue->Lookup("ByteRange"));
+    if (!pByteRange || pByteRange->GetElements().size() < 2)
+    {
+        return false;
+    }
+
+    std::vector<std::pair<size_t, size_t>> aByteRanges;
+    if (!GetByteRangesFromPDF(*pByteRange, aByteRanges))
+    {
+        return false;
+    }
+
+    rEOF = aByteRanges[1].first + aByteRanges[1].second;
+    return true;
+}
+
+/// Checks if there are unsigned incremental updates between the signatures or after the last one.
+bool IsCompleteSignature(SvStream& rStream, vcl::filter::PDFDocument& rDocument,
+                         vcl::filter::PDFObjectElement* pSignature)
+{
+    std::set<size_t> aSignedEOFs;
+    for (const auto& i : rDocument.GetSignatureWidgets())
+    {
+        size_t nEOF = 0;
+        if (!GetEOFOfSignature(i, nEOF))
+        {
+            return false;
+        }
+
+        aSignedEOFs.insert(nEOF);
+    }
+
+    size_t nSignatureEOF = 0;
+    if (!GetEOFOfSignature(pSignature, nSignatureEOF))
+    {
+        return false;
+    }
+
+    const std::vector<size_t>& rAllEOFs = rDocument.GetEOFs();
+    bool bFoundOwn = false;
+    for (const auto& rEOF : rAllEOFs)
+    {
+        if (rEOF == nSignatureEOF)
+        {
+            bFoundOwn = true;
+            continue;
+        }
+
+        if (!bFoundOwn)
+        {
+            continue;
+        }
+
+        if (aSignedEOFs.find(rEOF) == aSignedEOFs.end())
+        {
+            // Unsigned incremental update found.
+            return false;
+        }
+    }
+
+    // Make sure we find the incremental update of the signature itself.
+    if (!bFoundOwn)
+    {
+        return false;
+    }
+
+    // No additional content after the last incremental update.
+    rStream.Seek(STREAM_SEEK_TO_END);
+    size_t nFileEnd = rStream.Tell();
+    return std::find(rAllEOFs.begin(), rAllEOFs.end(), nFileEnd) != rAllEOFs.end();
+}
+}
+
 namespace xmlsecurity
 {
 namespace pdfio
 {
 
-bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, bool bLast)
+bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature, SignatureInformation& rInformation, vcl::filter::PDFDocument& rDocument)
 {
     vcl::filter::PDFObjectElement* pValue = pSignature->LookupObject("V");
     if (!pValue)
@@ -136,24 +248,9 @@ bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignat
 
     // Build a list of offset-length pairs, representing the signed bytes.
     std::vector<std::pair<size_t, size_t>> aByteRanges;
-    size_t nByteRangeOffset = 0;
-    const std::vector<vcl::filter::PDFElement*>& rByteRangeElements = pByteRange->GetElements();
-    for (size_t i = 0; i < rByteRangeElements.size(); ++i)
+    if (!GetByteRangesFromPDF(*pByteRange, aByteRanges))
     {
-        auto pNumber = dynamic_cast<vcl::filter::PDFNumberElement*>(rByteRangeElements[i]);
-        if (!pNumber)
-        {
-            SAL_WARN("xmlsecurity.pdfio", "ValidateSignature: signature offset and length has to be a number");
-            return false;
-        }
-
-        if (i % 2 == 0)
-        {
-            nByteRangeOffset = pNumber->GetValue();
-            continue;
-        }
-        size_t nByteRangeLength = pNumber->GetValue();
-        aByteRanges.emplace_back(nByteRangeOffset, nByteRangeLength);
+        return false;
     }
 
     // Detect if the byte ranges don't cover everything, but the signature itself.
@@ -174,11 +271,7 @@ bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignat
         SAL_WARN("xmlsecurity.pdfio", "ValidateSignature: second range start is not the end of the signature");
         return false;
     }
-    rStream.Seek(STREAM_SEEK_TO_END);
-    size_t nFileEnd = rStream.Tell();
-    if (bLast && (aByteRanges[1].first + aByteRanges[1].second) != nFileEnd)
-        // Second range end is not the end of the file.
-        rInformation.bPartialDocumentSignature = true;
+    rInformation.bPartialDocumentSignature = !IsCompleteSignature(rStream, rDocument, pSignature);
 
     // At this point there is no obviously missing info to validate the
     // signature.
diff --git a/xmlsecurity/workben/pdfverify.cxx b/xmlsecurity/workben/pdfverify.cxx
index 55a20d51a6b9..7ddb2623944c 100644
--- a/xmlsecurity/workben/pdfverify.cxx
+++ b/xmlsecurity/workben/pdfverify.cxx
@@ -149,8 +149,8 @@ int pdfVerify(int nArgc, char** pArgv)
             for (size_t i = 0; i < aSignatures.size(); ++i)
             {
                 SignatureInformation aInfo(i);
-                bool bLast = i == aSignatures.size() - 1;
-                if (!xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[i], aInfo, bLast))
+                if (!xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[i], aInfo,
+                                                           aDocument))
                 {
                     SAL_WARN("xmlsecurity.pdfio", "failed to determine digest match");
                     return 1;
@@ -158,6 +158,8 @@ int pdfVerify(int nArgc, char** pArgv)
 
                 bool bSuccess = aInfo.nStatus == xml::crypto::SecurityOperationStatus_OPERATION_SUCCEEDED;
                 std::cerr << "signature #" << i << ": digest match? " << bSuccess << std::endl;
+                std::cerr << "signature #" << i << ": partial? " << aInfo.bPartialDocumentSignature
+                          << std::endl;
             }
         }