[Libreoffice-commits] core.git: xmlsecurity/qa xmlsecurity/source

[Miklos Vajna (via logerrit)]

 xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p3-stamp.pdf |binary
 xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx             |   15 ++++
 xmlsecurity/source/helper/pdfsignaturehelper.cxx          |   49 ++++++++++++--
 3 files changed, 59 insertions(+), 5 deletions(-)

New commits:
commit f231dacde9df1c4aa5f4e0970535c4f4093364a7
Author:     Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Wed Nov 4 21:39:04 2020 +0100
Commit:     Miklos Vajna <vmiklos at collabora.com>
CommitDate: Thu Nov 5 09:04:29 2020 +0100

    xmlsecurity: reject a few dangerous annotation types during pdf sig verify
    
    Change-Id: I950b49a6e7181639daf27348ddfa0f36586baa65
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105312
    Tested-by: Jenkins
    Reviewed-by: Miklos Vajna <vmiklos at collabora.com>

diff --git a/xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p3-stamp.pdf b/xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p3-stamp.pdf
new file mode 100644
index 000000000000..b30f5b03867c
Binary files /dev/null and b/xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p3-stamp.pdf differ
diff --git a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
index 78c564b26e28..fb47b9887f15 100644
--- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
+++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
@@ -402,6 +402,21 @@ CPPUNIT_TEST_FIXTURE(PDFSigningTest, testBadCertP1)
                          rInformation.nStatus);
 }
 
+CPPUNIT_TEST_FIXTURE(PDFSigningTest, testBadCertP3Stamp)
+{
+    std::vector<SignatureInformation> aInfos
+        = verify(m_directories.getURLFromSrc(DATA_DIRECTORY) + "bad-cert-p3-stamp.pdf", 1);
+    CPPUNIT_ASSERT(!aInfos.empty());
+    SignatureInformation& rInformation = aInfos[0];
+
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 0 (SecurityOperationStatus_UNKNOWN)
+    // - Actual  : 1 (SecurityOperationStatus_OPERATION_SUCCEEDED)
+    // i.e. adding a stamp annotation was not considered as a bad modification.
+    CPPUNIT_ASSERT_EQUAL(xml::crypto::SecurityOperationStatus::SecurityOperationStatus_UNKNOWN,
+                         rInformation.nStatus);
+}
+
 /// 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 f7427337fd1e..72f7618a2253 100644
--- a/xmlsecurity/source/helper/pdfsignaturehelper.cxx
+++ b/xmlsecurity/source/helper/pdfsignaturehelper.cxx
@@ -226,8 +226,30 @@ bool IsCompleteSignature(SvStream& rStream, vcl::filter::PDFDocument& rDocument,
 }
 
 #if HAVE_FEATURE_PDFIUM
+
+/**
+ * Contains checksums of a PDF page, which is rendered without annotations. It also contains
+ * the geometry of a few dangerous annotation types.
+ */
+struct PageChecksum
+{
+    BitmapChecksum m_nPageContent;
+    std::vector<basegfx::B2DRectangle> m_aAnnotations;
+    bool operator==(const PageChecksum& rChecksum) const;
+};
+
+bool PageChecksum::operator==(const PageChecksum& rChecksum) const
+{
+    if (m_nPageContent != rChecksum.m_nPageContent)
+    {
+        return false;
+    }
+
+    return m_aAnnotations == rChecksum.m_aAnnotations;
+}
+
 /// Collects the checksum of each page of one version of the PDF.
-void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vector<BitmapChecksum>& rPageChecksums,
+void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vector<PageChecksum>& rPageChecksums,
                              int nMDPPerm)
 {
     auto pPdfium = vcl::pdf::PDFiumLibrary::get();
@@ -247,8 +269,25 @@ void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vector<BitmapChecksum
             return;
         }
 
-        BitmapChecksum nPageChecksum = pPdfPage->getChecksum(nMDPPerm);
-        rPageChecksums.push_back(nPageChecksum);
+        PageChecksum aPageChecksum;
+        aPageChecksum.m_nPageContent = pPdfPage->getChecksum(nMDPPerm);
+        for (int i = 0; i < pPdfPage->getAnnotationCount(); ++i)
+        {
+            std::unique_ptr<vcl::pdf::PDFiumAnnotation> pPdfAnnotation = pPdfPage->getAnnotation(i);
+            vcl::pdf::PDFAnnotationSubType eType = pPdfAnnotation->getSubType();
+            switch (eType)
+            {
+                case vcl::pdf::PDFAnnotationSubType::Unknown:
+                case vcl::pdf::PDFAnnotationSubType::FreeText:
+                case vcl::pdf::PDFAnnotationSubType::Stamp:
+                case vcl::pdf::PDFAnnotationSubType::Redact:
+                    aPageChecksum.m_aAnnotations.push_back(pPdfAnnotation->getRectangle());
+                    break;
+                default:
+                    break;
+            }
+        }
+        rPageChecksums.push_back(aPageChecksum);
     }
 }
 #endif
@@ -272,7 +311,7 @@ bool IsValidSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignatu
     aSignatureStream.WriteStream(rStream, nSignatureEOF);
     rStream.Seek(nPos);
     aSignatureStream.Seek(0);
-    std::vector<BitmapChecksum> aSignedPages;
+    std::vector<PageChecksum> aSignedPages;
     AnalyizeSignatureStream(aSignatureStream, aSignedPages, nMDPPerm);
 
     SvMemoryStream aFullStream;
@@ -281,7 +320,7 @@ bool IsValidSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignatu
     aFullStream.WriteStream(rStream);
     rStream.Seek(nPos);
     aFullStream.Seek(0);
-    std::vector<BitmapChecksum> aAllPages;
+    std::vector<PageChecksum> aAllPages;
     AnalyizeSignatureStream(aFullStream, aAllPages, nMDPPerm);
 
     // Fail if any page looks different after signing and at the end. Annotations/commenting doesn't