xmlsecurity: detect unsigned incremental update between signatures

(cherry picked from commit 7468d5df5ec79783eae84b62bdc5ecf12f0ca255)

Conflicts:
	vcl/source/filter/ipdf/pdfdocument.cxx
	xmlsecurity/source/pdfio/pdfdocument.cxx

Change-Id: I269ed858852ee7d1275adf340c8cc1565fc30693
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99382
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
(cherry picked from commit ed21fa8d6edd97fad3a5b606b3522ab3bff14e05)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/99838
Tested-by: Gabor Kelemen <kelemen.gabor2@nisz.hu>
Reviewed-by: Gabor Kelemen <kelemen.gabor2@nisz.hu>

8 files changed, 153 insertions, 34 deletions

diff --git a/include/vcl/filter/pdfdocument.hxx b/include/vcl/filter/pdfdocument.hxx
index 65e84880c789..e05e7d9d749c 100644
--- a/include/vcl/filter/pdfdocument.hxx
+++ b/include/vcl/filter/pdfdocument.hxx
@@ -426,6 +426,8 @@ public:
     std::vector<PDFObjectElement*> GetSignatureWidgets();
     /// Remove the nth signature from read document in the edit buffer.
     bool RemoveSignature(size_t nPosition);
+    /// Get byte offsets of the end of incremental updates.
+    const std::vector<size_t>& GetEOFs() const;
     //@}
 };
 
diff --git a/vcl/source/filter/ipdf/pdfdocument.cxx b/vcl/source/filter/ipdf/pdfdocument.cxx
index 02bde3fcaedb..1afc3167ae91 100644
--- a/vcl/source/filter/ipdf/pdfdocument.cxx
+++ b/vcl/source/filter/ipdf/pdfdocument.cxx
@@ -147,6 +147,8 @@ bool PDFDocument::RemoveSignature(size_t nPosition)
     return m_aEditBuffer.good();
 }
 
+const std::vector<size_t>& PDFDocument::GetEOFs() const { return m_aEOFs; }
+
 sal_uInt32 PDFDocument::GetNextSignature()
 {
     sal_uInt32 nRet = 0;
@@ -1979,7 +1981,16 @@ bool PDFCommentElement::Read(SvStream& rStream)
             m_aComment = aBuf.makeStringAndClear();
 
             if (m_aComment.startsWith("%%EOF"))
-                m_rDoc.PushBackEOF(rStream.Tell());
+            {
+                sal_uInt64 nPos = rStream.Tell();
+                if (ch == '\r')
+                {
+                    // If the comment ends with a \r\n, count the \n as well to match Adobe Acrobat
+                    // behavior.
+                    nPos += 1;
+                }
+                m_rDoc.PushBackEOF(nPos);
+            }
 
             SAL_INFO("vcl.filter", "PDFCommentElement::Read: m_aComment is '" << m_aComment << "'");
             return true;
diff --git a/xmlsecurity/inc/pdfio/pdfdocument.hxx b/xmlsecurity/inc/pdfio/pdfdocument.hxx
index 996bb1527bb8..f7e36492e746 100644
--- a/xmlsecurity/inc/pdfio/pdfdocument.hxx
+++ b/xmlsecurity/inc/pdfio/pdfdocument.hxx
@@ -18,6 +18,7 @@ namespace vcl
 namespace filter
 {
 class PDFObjectElement;
+class PDFDocument;
 }
 }
 struct SignatureInformation;
@@ -29,12 +30,13 @@ 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);
+                                             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 97e0b7d28f97..8a7cbbdc3730 100644
--- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
+++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
@@ -96,9 +96,8 @@ 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));
+            xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[i], aInfo, aVerifyDocument));
         aRet.push_back(aInfo);
 
         if (!rExpectedSubFilter.isEmpty())
@@ -243,7 +242,7 @@ CPPUNIT_TEST_FIXTURE(PDFSigningTest, testPDFRemove)
         CPPUNIT_ASSERT_EQUAL(static_cast<size_t>(1), aSignatures.size());
         SignatureInformation aInfo(0);
         CPPUNIT_ASSERT(
-            xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[0], aInfo, /*bLast=*/true));
+            xmlsecurity::pdfio::ValidateSignature(aStream, aSignatures[0], aInfo, aDocument));
     }
 
     // Remove the signature and write out the result as remove.pdf.
@@ -399,6 +398,18 @@ CPPUNIT_TEST_FIXTURE(PDFSigningTest, testPartial)
     CPPUNIT_ASSERT(rInformation.bPartialDocumentSignature);
 }
 
+CPPUNIT_TEST_FIXTURE(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);
+}
+
 /// Test writing a PAdES signature.
 CPPUNIT_TEST_FIXTURE(PDFSigningTest, testSigningCertificateAttribute)
 {
diff --git a/xmlsecurity/source/helper/pdfsignaturehelper.cxx b/xmlsecurity/source/helper/pdfsignaturehelper.cxx
index 809e089b0f4f..f10f29c61840 100644
--- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx
+++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx
@@ -56,8 +56,7 @@ bool PDFSignatureHelper::ReadAndVerifySignature(
     {
         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 febcd2e3a1ea..7cf2c137c1c4 100644
--- a/xmlsecurity/source/pdfio/pdfdocument.cxx
+++ b/xmlsecurity/source/pdfio/pdfdocument.cxx
@@ -23,12 +23,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)
+                       SignatureInformation& rInformation, vcl::filter::PDFDocument& rDocument)
 {
     vcl::filter::PDFObjectElement* pValue = pSignature->LookupObject("V");
     if (!pValue)
@@ -110,25 +222,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.
@@ -150,11 +246,7 @@ bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignat
                  "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 191ca8c087eb..3076d1c47a43 100644
--- a/xmlsecurity/workben/pdfverify.cxx
+++ b/xmlsecurity/workben/pdfverify.cxx
@@ -159,8 +159,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;
@@ -169,6 +169,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;
             }
         }