summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMiklos Vajna <vmiklos@collabora.com>2020-11-04 21:39:04 +0100
committerMiklos Vajna <vmiklos@collabora.com>2020-12-18 16:52:07 +0100
commit4c04745a2deee6590970d5139be958817b8b3591 (patch)
tree0fb5908d3ab2189656c697a5eecf97e4b5d56e1c
parent255ff5f6f9f11f72fca48b337c8dc6f2d08e8d6b (diff)
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 <jenkinscollaboraoffice@gmail.com> Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
-rw-r--r--xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p3-stamp.pdfbin0 -> 22023 bytes
-rw-r--r--xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx18
-rw-r--r--xmlsecurity/source/pdfio/pdfdocument.cxx63
3 files changed, 76 insertions, 5 deletions
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
--- /dev/null
+++ b/xmlsecurity/qa/unit/pdfsigning/data/bad-cert-p3-stamp.pdf
Binary files 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<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);
+}
+
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 <svl/sigstruct.hxx>
#include <svl/cryptosign.hxx>
#include <vcl/bitmap.hxx>
+#include <basegfx/range/b2drectangle.hxx>
+
+#if HAVE_FEATURE_PDFIUM
+#include <fpdf_annot.h>
+#endif
#ifdef XMLSEC_CRYPTO_NSS
#include <cert.h>
@@ -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<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
@@ -185,8 +211,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);
}
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<BitmapChecksum> aSignedPages;
+ std::vector<PageChecksum> 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<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