From 4c04745a2deee6590970d5139be958817b8b3591 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Wed, 4 Nov 2020 21:39:04 +0100 Subject: xmlsecurity: reject a few dangerous annotation types during pdf sig verify (cherry picked from commit f231dacde9df1c4aa5f4e0970535c4f4093364a7) Conflicts: include/vcl/filter/PDFiumLibrary.hxx xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx xmlsecurity/source/helper/pdfsignaturehelper.cxx xmlsecurity/source/pdfio/pdfdocument.cxx Change-Id: I950b49a6e7181639daf27348ddfa0f36586baa65 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/107969 Tested-by: Jenkins CollaboraOffice Reviewed-by: Miklos Vajna --- .../qa/unit/pdfsigning/data/bad-cert-p3-stamp.pdf | Bin 0 -> 22023 bytes xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx | 18 ++++++ xmlsecurity/source/pdfio/pdfdocument.cxx | 63 +++++++++++++++++++-- 3 files changed, 76 insertions(+), 5 deletions(-) create mode 100644 xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p3-stamp.pdf 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 899a7567c4a3..91e565cf2813 100644 --- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx +++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx @@ -73,6 +73,7 @@ public: void testPartial(); void testPartialInBetween(); void testBadCertP1(); + void testBadCertP3Stamp(); /// Test writing a PAdES signature. void testSigningCertificateAttribute(); /// Test that we accept files which are supposed to be good. @@ -96,6 +97,7 @@ public: CPPUNIT_TEST(testPartial); CPPUNIT_TEST(testPartialInBetween); CPPUNIT_TEST(testBadCertP1); + CPPUNIT_TEST(testBadCertP3Stamp); CPPUNIT_TEST(testSigningCertificateAttribute); CPPUNIT_TEST(testGood); CPPUNIT_TEST(testTokenize); @@ -419,6 +421,22 @@ void PDFSigningTest::testBadCertP1() rInformation.nStatus); } +void PDFSigningTest::testBadCertP3Stamp() +{ + std::vector 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); +} + void PDFSigningTest::testSigningCertificateAttribute() { // Create a new signature. diff --git a/xmlsecurity/source/pdfio/pdfdocument.cxx b/xmlsecurity/source/pdfio/pdfdocument.cxx index 7b697d6d86eb..160bbee6bcde 100644 --- a/xmlsecurity/source/pdfio/pdfdocument.cxx +++ b/xmlsecurity/source/pdfio/pdfdocument.cxx @@ -38,6 +38,11 @@ #include #include #include +#include + +#if HAVE_FEATURE_PDFIUM +#include +#endif #ifdef XMLSEC_CRYPTO_NSS #include @@ -167,8 +172,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 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& rPageChecksums, +void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vector& rPageChecksums, int nMDPPerm) { #if HAVE_FEATURE_PDFIUM @@ -185,8 +211,35 @@ void AnalyizeSignatureStream(SvMemoryStream& rStream, std::vectorgetChecksum(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); } FPDF_CloseDocument(pPdfDocument); #else @@ -212,7 +265,7 @@ bool IsValidSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignatu aSignatureStream.WriteStream(rStream, nSignatureEOF); rStream.Seek(nPos); aSignatureStream.Seek(0); - std::vector aSignedPages; + std::vector aSignedPages; AnalyizeSignatureStream(aSignatureStream, aSignedPages, nMDPPerm); SvMemoryStream aFullStream; @@ -221,7 +274,7 @@ bool IsValidSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignatu aFullStream.WriteStream(rStream); rStream.Seek(nPos); aFullStream.Seek(0); - std::vector aAllPages; + std::vector aAllPages; AnalyizeSignatureStream(aFullStream, aAllPages, nMDPPerm); // Fail if any page looks different after signing and at the end. Annotations/commenting doesn't -- cgit