[Libreoffice-commits] core.git: Branch 'feature/cib_contract891c' - 2 commits - configure.ac include/tools include/vcl tools/source vcl/source xmlsecurity/Library_xmlsecurity.mk xmlsecurity/qa xmlsecurity/source xmlsecurity/workben
Samuel Mehrbrodt (via logerrit)
logerrit at kemper.freedesktop.org
Thu Oct 1 08:45:53 UTC 2020
Rebased ref, commits from common ancestor:
commit 3ac43088158476d32701d00e7d517440b128c1a4
Author: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
AuthorDate: Tue Sep 29 09:22:04 2020 +0200
Commit: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
CommitDate: Thu Oct 1 10:45:07 2020 +0200
Release 5.4.11
Change-Id: I94f4cb91b1cf92722ff43d3561ba0cf2405a6a29
diff --git a/configure.ac b/configure.ac
index 83fe089baf59..af7fc01b8195 100644
--- a/configure.ac
+++ b/configure.ac
@@ -9,7 +9,7 @@ dnl in order to create a configure script.
# several non-alphanumeric characters, those are split off and used only for the
# ABOUTBOXPRODUCTVERSIONSUFFIX in openoffice.lst. Why that is necessary, no idea.
-AC_INIT([LibreOffice],[5.4.10.0],[],[],[http://documentfoundation.org/])
+AC_INIT([LibreOffice],[5.4.11.0],[],[],[http://documentfoundation.org/])
AC_PREREQ([2.59])
commit fba3e52b85c2d4d4a5e2f7394c4fe6f3a70405b9
Author: Miklos Vajna <vmiklos at collabora.com>
AuthorDate: Fri Sep 4 17:17:48 2020 +0200
Commit: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
CommitDate: Thu Oct 1 10:45:03 2020 +0200
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)
Conflicts:
include/vcl/filter/PDFiumLibrary.hxx
vcl/source/pdf/PDFiumLibrary.cxx
Conflicts:
xmlsecurity/qa/unit/signing/signing.cxx
Change-Id: I4607c242b3c6f6b01517b02407e9e7a095e2e069
diff --git a/include/tools/stream.hxx b/include/tools/stream.hxx
index 0bc3766807fa..608f7f0adde0 100644
--- a/include/tools/stream.hxx
+++ b/include/tools/stream.hxx
@@ -257,6 +257,7 @@ public:
SvStream& WriteOString(const OString& rStr)
{ return WriteCharPtr(rStr.getStr()); }
SvStream& WriteStream( SvStream& rStream );
+ sal_uInt64 WriteStream( SvStream& rStream, sal_uInt64 nSize );
SvStream& WriteBool( bool b )
{ return WriteUChar(static_cast<unsigned char>(b)); }
diff --git a/include/vcl/filter/PDFiumLibrary.hxx b/include/vcl/filter/PDFiumLibrary.hxx
index b9bceabb8acf..ffc70874c19b 100644
--- a/include/vcl/filter/PDFiumLibrary.hxx
+++ b/include/vcl/filter/PDFiumLibrary.hxx
@@ -17,11 +17,16 @@
#include <memory>
#include <rtl/instance.hxx>
#include <vcl/dllapi.h>
+#include <vcl/checksum.hxx>
+
+#include <fpdf_doc.h>
namespace vcl
{
namespace pdf
{
+class PDFiumDocument;
+
class VCL_DLLPUBLIC PDFium final
{
private:
@@ -33,6 +38,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/tools/source/stream/stream.cxx b/tools/source/stream/stream.cxx
index 488348719892..b83729e35fbf 100644
--- a/tools/source/stream/stream.cxx
+++ b/tools/source/stream/stream.cxx
@@ -1176,6 +1176,27 @@ SvStream& SvStream::WriteStream( SvStream& rStream )
return *this;
}
+sal_uInt64 SvStream::WriteStream( SvStream& rStream, sal_uInt64 nSize )
+{
+ const sal_uInt32 cBufLen = 0x8000;
+ std::unique_ptr<char[]> pBuf( new char[ cBufLen ] );
+ sal_uInt32 nCurBufLen = cBufLen;
+ sal_uInt32 nCount;
+ sal_uInt64 nWriteSize = nSize;
+
+ do {
+ if ( nSize >= nCurBufLen )
+ nWriteSize -= nCurBufLen;
+ else
+ nCurBufLen = nWriteSize;
+ nCount = rStream.ReadBytes( pBuf.get(), nCurBufLen );
+ WriteBytes( pBuf.get(), nCount );
+ }
+ while( nWriteSize && nCount == nCurBufLen );
+
+ return nSize - nWriteSize;
+}
+
OUString SvStream::ReadUniOrByteString( rtl_TextEncoding eSrcCharSet )
{
// read UTF-16 string directly from stream ?
diff --git a/vcl/source/pdf/PDFiumLibrary.cxx b/vcl/source/pdf/PDFiumLibrary.cxx
index 5f487b15f48b..38eb88a99db0 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 <o3tl/make_unique.hxx>
+#include <vcl/bitmap.hxx>
+#include <vcl/bitmapaccess.hxx>
+
namespace vcl
{
namespace pdf
@@ -31,6 +35,57 @@ 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 = o3tl::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);
+ {
+ Bitmap::ScopedWriteAccess pWriteAccess(aBitmap);
+ const auto pPdfBuffer = static_cast<const sal_uInt8*>(FPDFBitmap_GetBuffer(pPdfBitmap));
+ const int nStride = FPDFBitmap_GetStride(pPdfBitmap);
+ for (size_t nRow = 0; nRow < nPageHeight; ++nRow)
+ {
+ const sal_uInt8* pPdfLine = pPdfBuffer + (nStride * nRow);
+ pWriteAccess->CopyScanline(nRow, pPdfLine, ScanlineFormat::N32BitTcBgra, nStride);
+ }
+ }
+ return aBitmap.GetChecksum();
+}
}
} // end vcl::pdf
diff --git a/xmlsecurity/Library_xmlsecurity.mk b/xmlsecurity/Library_xmlsecurity.mk
index 77d3bd81dc3b..85950d1dcd4b 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,$(SRCDIR)/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
Binary files /dev/null and b/xmlsecurity/qa/unit/signing/data/hide-and-replace-shadow-file-signed-2.pdf differ
diff --git a/xmlsecurity/qa/unit/signing/signing.cxx b/xmlsecurity/qa/unit/signing/signing.cxx
index 2b6e60e7c0bd..29e35738c62b 100644
--- a/xmlsecurity/qa/unit/signing/signing.cxx
+++ b/xmlsecurity/qa/unit/signing/signing.cxx
@@ -88,6 +88,8 @@ public:
void testPDFGood();
/// Test a typical PDF where the signature is bad.
void testPDFBad();
+ /// Test a maliciously manipulated signed pdf
+ void testPDFHideAndReplace();
/// Test a typical PDF which is not signed.
void testPDFNo();
#endif
@@ -113,6 +115,7 @@ public:
#if HAVE_FEATURE_PDFIMPORT
CPPUNIT_TEST(testPDFGood);
CPPUNIT_TEST(testPDFBad);
+ CPPUNIT_TEST(testPDFHideAndReplace);
CPPUNIT_TEST(testPDFNo);
#endif
CPPUNIT_TEST(test96097Calc);
@@ -458,6 +461,22 @@ void SigningTest::testPDFBad()
CPPUNIT_ASSERT_EQUAL(static_cast<int>(SignatureState::BROKEN), static_cast<int>(pObjectShell->GetDocumentSignatureState()));
}
+void 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()));
+}
+
void SigningTest::testPDFNo()
{
createDoc(m_directories.getURLFromSrc(DATA_DIRECTORY) + "no.pdf");
diff --git a/xmlsecurity/source/pdfio/pdfdocument.cxx b/xmlsecurity/source/pdfio/pdfdocument.cxx
index 5cec868a012b..edcb72d9a9ad 100644
--- a/xmlsecurity/source/pdfio/pdfdocument.cxx
+++ b/xmlsecurity/source/pdfio/pdfdocument.cxx
@@ -21,11 +21,15 @@
#include <filter/msfilter/mscodec.hxx>
#include <rtl/character.hxx>
#include <rtl/strbuf.hxx>
+#include <config_features.h>
+
+#include <vcl/filter/PDFiumLibrary.hxx>
#include <rtl/string.hxx>
#include <sal/log.hxx>
#include <sal/types.h>
#include <sax/tools/converter.hxx>
#include <tools/zcodec.hxx>
+#include <tools/stream.hxx>
#include <unotools/calendarwrapper.hxx>
#include <unotools/datetime.hxx>
#include <vcl/pdfwriter.hxx>
@@ -49,6 +53,8 @@
#include <comphelper/windowserrorstring.hxx>
#endif
+#include <vcl/bitmap.hxx>
+
using namespace com::sun::star;
namespace
@@ -392,6 +398,66 @@ bool VerifyNonDetachedSignature(SvStream& rStream, std::vector<std::pair<size_t,
return false;
}
#endif
+
+/// 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;
+}
}
bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignature,
@@ -499,6 +565,12 @@ bool ValidateSignature(SvStream& rStream, vcl::filter::PDFObjectElement* pSignat
}
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.
std::vector<unsigned char> aSignature = vcl::filter::PDFDocument::DecodeHexString(pContents);
diff --git a/xmlsecurity/workben/pdfverify.cxx b/xmlsecurity/workben/pdfverify.cxx
index ea48350246a6..d0b5405b015b 100644
--- a/xmlsecurity/workben/pdfverify.cxx
+++ b/xmlsecurity/workben/pdfverify.cxx
@@ -20,6 +20,7 @@
#include <vcl/pngwrite.hxx>
#include <vcl/svapp.hxx>
#include <vcl/graphicfilter.hxx>
+#include <comphelper/scopeguard.hxx>
#include <xmlsecurity/pdfio/pdfdocument.hxx>
@@ -72,11 +73,11 @@ int pdfVerify(int nArgc, char** pArgv)
uno::Reference<lang::XMultiServiceFactory> xMultiServiceFactory(xMultiComponentFactory, 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;
}
More information about the Libreoffice-commits
mailing list