From 4ebc328779539d9bc0cb0d7759c27e1e155c129e Mon Sep 17 00:00:00 2001
From: Miklos Vajna <vmiklos@collabora.com>
Date: Wed, 4 Nov 2020 21:39:04 +0100
Subject: xmlsecurity: reject a few dangerous annotation types during pdf sig
 verify
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

(cherry picked from commit f231dacde9df1c4aa5f4e0970535c4f4093364a7)

Conflicts:
	xmlsecurity/source/helper/pdfsignaturehelper.cxx

Reviewed-on: https://gerrit.libreoffice.org/c/core/+/105926
Tested-by: Jenkins
Reviewed-by: Caolán McNamara <caolanm@redhat.com>
(cherry picked from commit fcab45e0e22f4cf46e71856dba7ae5abd6f99bc5)

Change-Id: I950b49a6e7181639daf27348ddfa0f36586baa65

Conflicts:
	xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
(cherry-picked from 363997c76749219b900f47043d1b17ba8ec9bccd)

Change-Id: I7096222bc2547046d988e0ba28df725859270790
---
 external/liborcus/inc/pch/precompiled_orcus.hxx    | 109 +++++++++++++++++++++
 include/vcl/filter/PDFiumLibrary.hxx               |   2 +
 .../qa/unit/pdfsigning/data/bad-cert-p3-stamp.pdf  | Bin 0 -> 22023 bytes
 xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx      |  16 +++
 xmlsecurity/source/pdfio/pdfdocument.cxx           |  63 +++++++++++-
 5 files changed, 185 insertions(+), 5 deletions(-)
 create mode 100644 external/liborcus/inc/pch/precompiled_orcus.hxx
 create mode 100644 xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p3-stamp.pdf

diff --git a/external/liborcus/inc/pch/precompiled_orcus.hxx b/external/liborcus/inc/pch/precompiled_orcus.hxx
new file mode 100644
index 000000000000..28e1bc8119d8
--- /dev/null
+++ b/external/liborcus/inc/pch/precompiled_orcus.hxx
@@ -0,0 +1,109 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+/*
+ This file has been autogenerated by update_pch.sh. It is possible to edit it
+ manually (such as when an include file has been moved/renamed/removed). All such
+ manual changes will be rewritten by the next run of update_pch.sh (which presumably
+ also fixes all possible problems, so it's usually better to use it).
+
+ Generated on 2020-02-01 12:22:35 using:
+ ./bin/update_pch external/liborcus orcus --cutoff=1 --exclude:system --include:module --include:local
+
+ If after updating build fails, use the following command to locate conflicting headers:
+ ./bin/update_pch_bisect ./external/liborcus/inc/pch/precompiled_orcus.hxx "make external/liborcus.build" --find-conflicts
+*/
+
+#if PCH_LEVEL >= 1
+#include <algorithm>
+#include <cassert>
+#include <codecvt>
+#include <cstdio>
+#include <cstdlib>
+#include <cstring>
+#include <deque>
+#include <fstream>
+#include <functional>
+#include <iostream>
+#include <iterator>
+#include <limits>
+#include <locale>
+#include <map>
+#include <memory>
+#include <ostream>
+#include <sstream>
+#include <string>
+#include <unordered_map>
+#include <unordered_set>
+#include <vector>
+#include <boost/current_function.hpp>
+#include <boost/filesystem.hpp>
+#include <boost/iostreams/filter/gzip.hpp>
+#include <boost/iostreams/filtering_stream.hpp>
+#include <boost/optional.hpp>
+#include <boost/pool/object_pool.hpp>
+#endif // PCH_LEVEL >= 1
+#if PCH_LEVEL >= 2
+#endif // PCH_LEVEL >= 2
+#if PCH_LEVEL >= 3
+#include <mdds/global.hpp>
+#include <mdds/sorted_string_map.hpp>
+#include <orcus/config.hpp>
+#include <orcus/css_document_tree.hpp>
+#include <orcus/css_parser.hpp>
+#include <orcus/css_selector.hpp>
+#include <orcus/csv_parser.hpp>
+#include <orcus/dom_tree.hpp>
+#include <orcus/exception.hpp>
+#include <orcus/format_detection.hpp>
+#include <orcus/global.hpp>
+#include <orcus/info.hpp>
+#include <orcus/interface.hpp>
+#include <orcus/json_document_tree.hpp>
+#include <orcus/json_global.hpp>
+#include <orcus/json_parser.hpp>
+#include <orcus/json_structure_tree.hpp>
+#include <orcus/measurement.hpp>
+#include <orcus/orcus_csv.hpp>
+#include <orcus/orcus_gnumeric.hpp>
+#include <orcus/orcus_import_ods.hpp>
+#include <orcus/orcus_import_xlsx.hpp>
+#include <orcus/orcus_json.hpp>
+#include <orcus/orcus_ods.hpp>
+#include <orcus/orcus_xls_xml.hpp>
+#include <orcus/orcus_xlsx.hpp>
+#include <orcus/orcus_xml.hpp>
+#include <orcus/parser_base.hpp>
+#include <orcus/parser_global.hpp>
+#include <orcus/pstring.hpp>
+#include <orcus/sax_ns_parser.hpp>
+#include <orcus/sax_parser.hpp>
+#include <orcus/sax_parser_base.hpp>
+#include <orcus/sax_token_parser.hpp>
+#include <orcus/spreadsheet/export_interface.hpp>
+#include <orcus/spreadsheet/import_interface.hpp>
+#include <orcus/spreadsheet/import_interface_pivot.hpp>
+#include <orcus/spreadsheet/import_interface_view.hpp>
+#include <orcus/spreadsheet/styles.hpp>
+#include <orcus/spreadsheet/types.hpp>
+#include <orcus/stream.hpp>
+#include <orcus/string_pool.hpp>
+#include <orcus/threaded_sax_token_parser.hpp>
+#include <orcus/tokens.hpp>
+#include <orcus/xml_namespace.hpp>
+#include <orcus/xml_structure_tree.hpp>
+#include <orcus/yaml_document_tree.hpp>
+#include <orcus/yaml_parser.hpp>
+#include <orcus/zip_archive.hpp>
+#include <orcus/zip_archive_stream.hpp>
+#endif // PCH_LEVEL >= 3
+#if PCH_LEVEL >= 4
+#endif // PCH_LEVEL >= 4
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/include/vcl/filter/PDFiumLibrary.hxx b/include/vcl/filter/PDFiumLibrary.hxx
index 783b9a6da8b4..027e4939fab1 100644
--- a/include/vcl/filter/PDFiumLibrary.hxx
+++ b/include/vcl/filter/PDFiumLibrary.hxx
@@ -59,6 +59,8 @@ public:
             FPDF_ClosePage(mpPage);
     }
 
+    FPDF_PAGE getPointer() { return mpPage; }
+
     /// Get bitmap checksum of the page, without annotations/commenting.
     BitmapChecksum getChecksum(int nMDPPerm);
 };
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 b35a1cd9a528..4eee048b52f9 100644
--- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
+++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx
@@ -456,6 +456,22 @@ void 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,
+                 /*rExpectedSubFilter=*/OString());
+    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.
 void PDFSigningTest::testSigningCertificateAttribute()
 {
diff --git a/xmlsecurity/source/pdfio/pdfdocument.cxx b/xmlsecurity/source/pdfio/pdfdocument.cxx
index 9d056de0a15c..51eac91495a7 100644
--- a/xmlsecurity/source/pdfio/pdfdocument.cxx
+++ b/xmlsecurity/source/pdfio/pdfdocument.cxx
@@ -24,6 +24,11 @@
 #include <svl/cryptosign.hxx>
 #include <vcl/filter/pdfdocument.hxx>
 #include <vcl/bitmap.hxx>
+#include <basegfx/range/b2drectangle.hxx>
+
+#if HAVE_FEATURE_PDFIUM
+#include <fpdf_annot.h>
+#endif
 
 using namespace com::sun::star;
 
@@ -138,8 +143,29 @@ bool IsCompleteSignature(SvStream& rStream, vcl::filter::PDFDocument& rDocument,
     return std::find(rAllEOFs.begin(), rAllEOFs.end(), nFileEnd) != rAllEOFs.end();
 }
 
+/**
+ * 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)
 {
 #if HAVE_FEATURE_PDFIUM
@@ -156,8 +182,35 @@ 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 < FPDFPage_GetAnnotCount(pPdfPage->getPointer()); ++i)
+        {
+            FPDF_ANNOTATION pAnnotation = FPDFPage_GetAnnot(pPdfPage->getPointer(), i);
+            int nType = FPDFAnnot_GetSubtype(pAnnotation);
+            switch (nType)
+            {
+                case FPDF_ANNOT_UNKNOWN:
+                case FPDF_ANNOT_FREETEXT:
+                case FPDF_ANNOT_STAMP:
+                case FPDF_ANNOT_REDACT:
+                {
+                    basegfx::B2DRectangle aB2DRectangle;
+                    FS_RECTF aRect;
+                    if (FPDFAnnot_GetRect(pAnnotation, &aRect))
+                    {
+                        aB2DRectangle = basegfx::B2DRectangle(aRect.left, aRect.top, aRect.right,
+                                                              aRect.bottom);
+                    }
+                    aPageChecksum.m_aAnnotations.push_back(aB2DRectangle);
+                    break;
+                }
+                default:
+                    break;
+            }
+            FPDFPage_CloseAnnot(pAnnotation);
+        }
+        rPageChecksums.push_back(aPageChecksum);
     }
 #else
     (void)rStream;
@@ -182,7 +235,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;
@@ -191,7 +244,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
-- 
cgit