xmlsecurity: pdf incremental updates that are non-commenting are invalid

I.e. it's OK to add incremental updates for annotation/commenting
purposes and that doesn't invalite existing signatures. Everything else
does.

(cherry picked from commit 61834cd574568613f0b0a2ee099a60fa5a8d9804)

[ Also disable a pdfium assert on Windows, only on this branch, where it
fails during CppunitTest_xmlsecurity_pdfsigning for reasons unclear to
me. ]

Conflicts:
	include/vcl/filter/PDFiumLibrary.hxx
	vcl/source/pdf/PDFiumLibrary.cxx

Change-Id: I4607c242b3c6f6b01517b02407e9e7a095e2e069
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/102325
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>

8 files changed, 211 insertions, 3 deletions

diff --git a/external/pdfium/build.patch.1 b/external/pdfium/build.patch.1
index 16544f0c7c81..b19fa1174419 100644
--- a/external/pdfium/build.patch.1
+++ b/external/pdfium/build.patch.1
@@ -37,3 +37,18 @@ index 0fb627ba8..dda1fc8bc 100644
        : span(container.data(), container.size()) {}
    template <
        typename Container,
+--- a/core/fxge/dib/cfx_cmyk_to_srgb.cpp	2020-09-10 17:32:37.165872018 +0200
++++ b/core/fxge/dib/cfx_cmyk_to_srgb.cpp	2020-09-10 17:33:15.870395738 +0200
+@@ -1740,10 +1740,12 @@
+   uint8_t y1 = static_cast<int>(y * 255.f + rounding_offset);
+   uint8_t k1 = static_cast<int>(k * 255.f + rounding_offset);
+ 
++#ifndef _WIN32
+   ASSERT(c1 == FXSYS_roundf(c * 255));
+   ASSERT(m1 == FXSYS_roundf(m * 255));
+   ASSERT(y1 == FXSYS_roundf(y * 255));
+   ASSERT(k1 == FXSYS_roundf(k * 255));
++#endif
+ 
+   uint8_t r;
+   uint8_t g;
diff --git a/include/vcl/filter/PDFiumLibrary.hxx b/include/vcl/filter/PDFiumLibrary.hxx
index bc7912c17e81..639c71d61a3d 100644
--- a/include/vcl/filter/PDFiumLibrary.hxx
+++ b/include/vcl/filter/PDFiumLibrary.hxx
@@ -17,9 +17,14 @@
 #include <memory>
 #include <rtl/instance.hxx>
 #include <vcl/dllapi.h>
+#include <vcl/checksum.hxx>
+
+#include <fpdf_doc.h>
 
 namespace vcl::pdf
 {
+class PDFiumDocument;
+
 class VCL_DLLPUBLIC PDFium final
 {
 private:
@@ -31,6 +36,49 @@ public:
     ~PDFium();
 };
 
+class VCL_DLLPUBLIC PDFiumPage final
+{
+private:
+    FPDF_PAGE mpPage;
+
+private:
+    PDFiumPage(const PDFiumPage&) = delete;
+    PDFiumPage& operator=(const PDFiumPage&) = delete;
+
+public:
+    PDFiumPage(FPDF_PAGE pPage)
+        : mpPage(pPage)
+    {
+    }
+
+    ~PDFiumPage()
+    {
+        if (mpPage)
+            FPDF_ClosePage(mpPage);
+    }
+
+    /// Get bitmap checksum of the page, without annotations/commenting.
+    BitmapChecksum getChecksum();
+};
+
+class VCL_DLLPUBLIC PDFiumDocument final
+{
+private:
+    FPDF_DOCUMENT mpPdfDocument;
+
+private:
+    PDFiumDocument(const PDFiumDocument&) = delete;
+    PDFiumDocument& operator=(const PDFiumDocument&) = delete;
+
+public:
+    PDFiumDocument(FPDF_DOCUMENT pPdfDocument);
+    ~PDFiumDocument();
+
+    int getPageCount();
+
+    std::unique_ptr<PDFiumPage> openPage(int nIndex);
+};
+
 struct PDFiumLibrary : public rtl::StaticWithInit<std::shared_ptr<PDFium>, PDFiumLibrary>
 {
     std::shared_ptr<PDFium> operator()() { return std::make_shared<PDFium>(); }
diff --git a/vcl/source/pdf/PDFiumLibrary.cxx b/vcl/source/pdf/PDFiumLibrary.cxx
index 604807524bf9..861b7dda0acb 100644
--- a/vcl/source/pdf/PDFiumLibrary.cxx
+++ b/vcl/source/pdf/PDFiumLibrary.cxx
@@ -15,6 +15,10 @@
 #include <vcl/filter/PDFiumLibrary.hxx>
 #include <fpdf_doc.h>
 
+#include <vcl/bitmap.hxx>
+
+#include <bitmapwriteaccess.hxx>
+
 namespace vcl::pdf
 {
 PDFium::PDFium()
@@ -29,6 +33,58 @@ PDFium::PDFium()
 
 PDFium::~PDFium() { FPDF_DestroyLibrary(); }
 
+PDFiumDocument::PDFiumDocument(FPDF_DOCUMENT pPdfDocument)
+    : mpPdfDocument(pPdfDocument)
+{
+}
+
+PDFiumDocument::~PDFiumDocument()
+{
+    if (mpPdfDocument)
+        FPDF_CloseDocument(mpPdfDocument);
+}
+
+std::unique_ptr<PDFiumPage> PDFiumDocument::openPage(int nIndex)
+{
+    std::unique_ptr<PDFiumPage> pPDFiumPage;
+    FPDF_PAGE pPage = FPDF_LoadPage(mpPdfDocument, nIndex);
+    if (pPage)
+    {
+        pPDFiumPage = std::make_unique<PDFiumPage>(pPage);
+    }
+    return pPDFiumPage;
+}
+
+int PDFiumDocument::getPageCount() { return FPDF_GetPageCount(mpPdfDocument); }
+
+BitmapChecksum PDFiumPage::getChecksum()
+{
+    size_t nPageWidth = FPDF_GetPageWidth(mpPage);
+    size_t nPageHeight = FPDF_GetPageHeight(mpPage);
+    FPDF_BITMAP pPdfBitmap = FPDFBitmap_Create(nPageWidth, nPageHeight, /*alpha=*/1);
+    if (!pPdfBitmap)
+    {
+        return 0;
+    }
+
+    // Intentionally not using FPDF_ANNOT here, annotations/commenting is OK to not affect the
+    // checksum, signature verification wants this.
+    FPDF_RenderPageBitmap(pPdfBitmap, mpPage, /*start_x=*/0, /*start_y=*/0, nPageWidth, nPageHeight,
+                          /*rotate=*/0, /*flags=*/0);
+    Bitmap aBitmap(Size(nPageWidth, nPageHeight), 24);
+    {
+        BitmapScopedWriteAccess pWriteAccess(aBitmap);
+        const auto pPdfBuffer = static_cast<ConstScanline>(FPDFBitmap_GetBuffer(pPdfBitmap));
+        const int nStride = FPDFBitmap_GetStride(pPdfBitmap);
+        for (size_t nRow = 0; nRow < nPageHeight; ++nRow)
+        {
+            ConstScanline pPdfLine = pPdfBuffer + (nStride * nRow);
+            pWriteAccess->CopyScanline(nRow, pPdfLine, ScanlineFormat::N32BitTcBgra, nStride);
+        }
+    }
+    return aBitmap.GetChecksum();
+}
+
 } // end vcl::pdf
 
 #endif // HAVE_FEATURE_PDFIUM
diff --git a/xmlsecurity/Library_xmlsecurity.mk b/xmlsecurity/Library_xmlsecurity.mk
index 038276c1725b..823332d3ec64 100644
--- a/xmlsecurity/Library_xmlsecurity.mk
+++ b/xmlsecurity/Library_xmlsecurity.mk
@@ -20,7 +20,10 @@ $(eval $(call gb_Library_add_defs,xmlsecurity,\
     -DXMLSECURITY_DLLIMPLEMENTATION \
 ))
 
-$(eval $(call gb_Library_use_externals,xmlsecurity,boost_headers))
+$(eval $(call gb_Library_use_externals,xmlsecurity,\
+	boost_headers \
+	$(if $(filter PDFIUM,$(BUILD_TYPE)),pdfium) \
+))
 
 $(eval $(call gb_Library_set_precompiled_header,xmlsecurity,xmlsecurity/inc/pch/precompiled_xmlsecurity))
 
diff --git a/xmlsecurity/qa/unit/signing/data/hide-and-replace-shadow-file-signed-2.pdf b/xmlsecurity/qa/unit/signing/data/hide-and-replace-shadow-file-signed-2.pdf
new file mode 100644
index 000000000000..f2b1a71096b2
--- /dev/null
+++ b/xmlsecurity/qa/unit/signing/data/hide-and-replace-shadow-file-signed-2.pdf
diff --git a/xmlsecurity/qa/unit/signing/signing.cxx b/xmlsecurity/qa/unit/signing/signing.cxx
index f2039b609e7e..df8ff85258b3 100644
--- a/xmlsecurity/qa/unit/signing/signing.cxx
+++ b/xmlsecurity/qa/unit/signing/signing.cxx
@@ -617,6 +617,22 @@ CPPUNIT_TEST_FIXTURE(SigningTest, testPDFBad)
                          static_cast<int>(pObjectShell->GetDocumentSignatureState()));
 }
 
+CPPUNIT_TEST_FIXTURE(SigningTest, testPDFHideAndReplace)
+{
+    createDoc(m_directories.getURLFromSrc(DATA_DIRECTORY)
+              + "hide-and-replace-shadow-file-signed-2.pdf");
+    SfxBaseModel* pBaseModel = dynamic_cast<SfxBaseModel*>(mxComponent.get());
+    CPPUNIT_ASSERT(pBaseModel);
+    SfxObjectShell* pObjectShell = pBaseModel->GetObjectShell();
+    CPPUNIT_ASSERT(pObjectShell);
+    // Without the accompanying fix in place, this test would have failed with:
+    // - Expected: 2 (BROKEN)
+    // - Actual  : 6 (NOTVALIDATED_PARTIAL_OK)
+    // i.e. a non-commenting update after a signature was not marked as invalid.
+    CPPUNIT_ASSERT_EQUAL(static_cast<int>(SignatureState::BROKEN),
+                         static_cast<int>(pObjectShell->GetDocumentSignatureState()));
+}
+
 /// Test a typical PDF which is not signed.
 CPPUNIT_TEST_FIXTURE(SigningTest, testPDFNo)
 {
diff --git a/xmlsecurity/source/pdfio/pdfdocument.cxx b/xmlsecurity/source/pdfio/pdfdocument.cxx
index 7cf2c137c1c4..557180071a2c 100644
--- a/xmlsecurity/source/pdfio/pdfdocument.cxx
+++ b/xmlsecurity/source/pdfio/pdfdocument.cxx
@@ -12,6 +12,9 @@
 #include <memory>
 #include <vector>
 
+#include <config_features.h>
+
+#include <vcl/filter/PDFiumLibrary.hxx>
 #include <rtl/string.hxx>
 #include <rtl/ustrbuf.hxx>
 #include <sal/log.hxx>
@@ -20,6 +23,7 @@
 #include <svl/sigstruct.hxx>
 #include <svl/cryptosign.hxx>
 #include <vcl/filter/pdfdocument.hxx>
+#include <vcl/bitmap.hxx>
 
 using namespace com::sun::star;
 
@@ -133,6 +137,66 @@ bool IsCompleteSignature(SvStream& rStream, vcl::filter::PDFDocument& rDocument,
     size_t nFileEnd = rStream.Tell();
     return std::find(rAllEOFs.begin(), rAllEOFs.end(), nFileEnd) != rAllEOFs.end();
 }
+
+/// Collects the checksum of each page of one version of the PDF.
+void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vector<BitmapChecksum>& rPageChecksums)
+{
+#if HAVE_FEATURE_PDFIUM
+    auto pPdfium = vcl::pdf::PDFiumLibrary::get();
+    vcl::pdf::PDFiumDocument aPdfDocument(
+        FPDF_LoadMemDocument(rStream.GetData(), rStream.GetSize(), /*password=*/nullptr));
+
+    int nPageCount = aPdfDocument.getPageCount();
+    for (int nPage = 0; nPage < nPageCount; ++nPage)
+    {
+        std::unique_ptr<vcl::pdf::PDFiumPage> pPdfPage(aPdfDocument.openPage(nPage));
+        if (!pPdfPage)
+        {
+            return;
+        }
+
+        BitmapChecksum nPageChecksum = pPdfPage->getChecksum();
+        rPageChecksums.push_back(nPageChecksum);
+    }
+#else
+    (void)rStream;
+#endif
+}
+
+/**
+ * Checks if incremental updates after singing performed valid modifications only.
+ * Annotations/commenting is OK, other changes are not.
+ */
+bool IsValidSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature)
+{
+    size_t nSignatureEOF = 0;
+    if (!GetEOFOfSignature(pSignature, nSignatureEOF))
+    {
+        return false;
+    }
+
+    SvMemoryStream aSignatureStream;
+    sal_uInt64 nPos = rStream.Tell();
+    rStream.Seek(0);
+    aSignatureStream.WriteStream(rStream, nSignatureEOF);
+    rStream.Seek(nPos);
+    aSignatureStream.Seek(0);
+    std::vector<BitmapChecksum> aSignedPages;
+    AnalyizeSignatureStream(aSignatureStream, aSignedPages);
+
+    SvMemoryStream aFullStream;
+    nPos = rStream.Tell();
+    rStream.Seek(0);
+    aFullStream.WriteStream(rStream);
+    rStream.Seek(nPos);
+    aFullStream.Seek(0);
+    std::vector<BitmapChecksum> aAllPages;
+    AnalyizeSignatureStream(aFullStream, aAllPages);
+
+    // Fail if any page looks different after signing and at the end. Annotations/commenting doesn't
+    // count, though.
+    return aSignedPages == aAllPages;
+}
 }
 
 namespace xmlsecurity
@@ -247,6 +311,11 @@ bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignat
         return false;
     }
     rInformation.bPartialDocumentSignature = !IsCompleteSignature(rStream, rDocument, pSignature);
+    if (!IsValidSignature(rStream, pSignature))
+    {
+        SAL_WARN("xmlsecurity.pdfio", "ValidateSignature: invalid incremental update detected");
+        return false;
+    }
 
     // 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 3076d1c47a43..b5052502573f 100644
--- a/xmlsecurity/workben/pdfverify.cxx
+++ b/xmlsecurity/workben/pdfverify.cxx
@@ -23,6 +23,7 @@
 #include <vcl/svapp.hxx>
 #include <vcl/graphicfilter.hxx>
 #include <vcl/filter/pdfdocument.hxx>
+#include <comphelper/scopeguard.hxx>
 
 #include <pdfio/pdfdocument.hxx>
 
@@ -80,11 +81,11 @@ int pdfVerify(int nArgc, char** pArgv)
                                                                     uno::UNO_QUERY);
     comphelper::setProcessServiceFactory(xMultiServiceFactory);
 
+    InitVCL();
+    comphelper::ScopeGuard g([] { DeInitVCL(); });
     if (nArgc > 3 && OString(pArgv[3]) == "-p")
     {
-        InitVCL();
         generatePreview(pArgv[1], pArgv[2]);
-        DeInitVCL();
         return 0;
     }