From 3e12ef2cbb54a48b1e925a427d5daa05e5f797e3 Mon Sep 17 00:00:00 2001 From: Miklos Vajna Date: Thu, 27 Oct 2016 13:00:27 +0200 Subject: xmlsecurity PDF verify: support array ref annotations Each pdf signature is mentioned in the Annots key of a page object. Usually the key contains an array value. But it's valid for the key to contain a reference to an object, where the object contains the actual array, so support this case as well. Also: - stop parsing name tokens on the first seen '(' character (usually there is a whitespace between the two, but that's not required) - handle \0 characters in the last 1024 bytes of the document by using std::search() instead of strstr(). Change-Id: I3a167b23340230f99f1ae4112473ed10e1c96b09 --- xmlsecurity/qa/unit/pdfsigning/data/pdf14adobe.pdf | Bin 0 -> 141054 bytes xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx | 63 +++++++++----- xmlsecurity/source/pdfio/pdfdocument.cxx | 93 +++++++++++++++++---- 3 files changed, 116 insertions(+), 40 deletions(-) create mode 100644 xmlsecurity/qa/unit/pdfsigning/data/pdf14adobe.pdf (limited to 'xmlsecurity') diff --git a/xmlsecurity/qa/unit/pdfsigning/data/pdf14adobe.pdf b/xmlsecurity/qa/unit/pdfsigning/data/pdf14adobe.pdf new file mode 100644 index 000000000000..58aadb492c07 Binary files /dev/null and b/xmlsecurity/qa/unit/pdfsigning/data/pdf14adobe.pdf differ diff --git a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx index 1f9ef8341810..62855c4527a6 100644 --- a/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx +++ b/xmlsecurity/qa/unit/pdfsigning/pdfsigning.cxx @@ -37,6 +37,12 @@ class PDFSigningTest : public test::BootstrapFixture * had nOriginalSignatureCount signatures. */ void sign(const OUString& rInURL, const OUString& rOutURL, size_t nOriginalSignatureCount); + /** + * Read a pdf and make sure that it has the expected number of valid + * signatures. + */ + void verify(const OUString& rURL, size_t nCount); + public: PDFSigningTest(); void setUp() override; @@ -49,12 +55,15 @@ public: void testPDFRemove(); /// Test removing all signatures from a previously multi-signed file. void testPDFRemoveAll(); + /// Test a PDF 1.4 document, signed by Adobe. + void testPDF14Adobe(); CPPUNIT_TEST_SUITE(PDFSigningTest); CPPUNIT_TEST(testPDFAdd); CPPUNIT_TEST(testPDFAdd2); CPPUNIT_TEST(testPDFRemove); CPPUNIT_TEST(testPDFRemoveAll); + CPPUNIT_TEST(testPDF14Adobe); CPPUNIT_TEST_SUITE_END(); }; @@ -81,6 +90,21 @@ void PDFSigningTest::setUp() #endif } +void PDFSigningTest::verify(const OUString& rURL, size_t nCount) +{ + SvFileStream aStream(rURL, StreamMode::READ); + xmlsecurity::pdfio::PDFDocument aVerifyDocument; + CPPUNIT_ASSERT(aVerifyDocument.Read(aStream)); + std::vector aSignatures = aVerifyDocument.GetSignatureWidgets(); + CPPUNIT_ASSERT_EQUAL(nCount, aSignatures.size()); + for (size_t i = 0; i < aSignatures.size(); ++i) + { + SignatureInformation aInfo(i); + bool bLast = i == aSignatures.size() - 1; + CPPUNIT_ASSERT(xmlsecurity::pdfio::PDFDocument::ValidateSignature(aStream, aSignatures[i], aInfo, bLast)); + } +} + void PDFSigningTest::sign(const OUString& rInURL, const OUString& rOutURL, size_t nOriginalSignatureCount) { // Make sure that input has nOriginalSignatureCount signatures. @@ -108,21 +132,8 @@ void PDFSigningTest::sign(const OUString& rInURL, const OUString& rOutURL, size_ CPPUNIT_ASSERT(aDocument.Write(aOutStream)); } - // Read back the signed pdf and make sure that it has one valid signature. - { - SvFileStream aStream(rOutURL, StreamMode::READ); - xmlsecurity::pdfio::PDFDocument aVerifyDocument; - CPPUNIT_ASSERT(aVerifyDocument.Read(aStream)); - std::vector aSignatures = aVerifyDocument.GetSignatureWidgets(); - // This was nOriginalSignatureCount when PDFDocument::Sign() silently returned success, without doing anything. - CPPUNIT_ASSERT_EQUAL(nOriginalSignatureCount + 1, aSignatures.size()); - for (size_t i = 0; i < aSignatures.size(); ++i) - { - SignatureInformation aInfo(i); - bool bLast = i == aSignatures.size() - 1; - CPPUNIT_ASSERT(xmlsecurity::pdfio::PDFDocument::ValidateSignature(aStream, aSignatures[i], aInfo, bLast)); - } - } + // This was nOriginalSignatureCount when PDFDocument::Sign() silently returned success, without doing anything. + verify(rOutURL, nOriginalSignatureCount + 1); } void PDFSigningTest::testPDFAdd() @@ -183,14 +194,9 @@ void PDFSigningTest::testPDFRemove() } // Read back the pdf and make sure that it no longer has signatures. - { - SvFileStream aStream(aOutURL, StreamMode::READ); - xmlsecurity::pdfio::PDFDocument aVerifyDocument; - CPPUNIT_ASSERT(aVerifyDocument.Read(aStream)); - std::vector aSignatures = aVerifyDocument.GetSignatureWidgets(); - // This failed when PDFDocument::RemoveSignature() silently returned success, without doing anything. - CPPUNIT_ASSERT(aSignatures.empty()); - } + // This failed when PDFDocument::RemoveSignature() silently returned + // success, without doing anything. + verify(aOutURL, 0); #endif } @@ -228,6 +234,17 @@ void PDFSigningTest::testPDFRemoveAll() CPPUNIT_ASSERT_EQUAL(static_cast(0), rInformations.size()); #endif } + +void PDFSigningTest::testPDF14Adobe() +{ +#ifndef _WIN32 + // Two signatures, first is SHA1, the second is SHA256. + // This was 0, as we failed to find the Annots key's value when it was a + // reference-to-array, not an array. + verify(m_directories.getURLFromSrc(DATA_DIRECTORY) + "pdf14adobe.pdf", 2); +#endif +} + CPPUNIT_TEST_SUITE_REGISTRATION(PDFSigningTest); CPPUNIT_PLUGIN_IMPLEMENT(); diff --git a/xmlsecurity/source/pdfio/pdfdocument.cxx b/xmlsecurity/source/pdfio/pdfdocument.cxx index df7a8091bcd7..adbf51bf893f 100644 --- a/xmlsecurity/source/pdfio/pdfdocument.cxx +++ b/xmlsecurity/source/pdfio/pdfdocument.cxx @@ -76,6 +76,7 @@ public: class PDFReferenceElement; class PDFDictionaryElement; +class PDFArrayElement; /// Indirect object: something with a unique ID. class PDFObjectElement : public PDFElement @@ -89,6 +90,8 @@ class PDFObjectElement : public PDFElement /// Length of the dictionary buffer till (before) the '<<' token. sal_uInt64 m_nDictionaryLength; PDFDictionaryElement* m_pDictionaryElement; + /// The contained direct array, if any. + PDFArrayElement* m_pArrayElement; public: PDFObjectElement(PDFDocument& rDoc, double fObjectValue, double fGenerationValue); @@ -103,6 +106,8 @@ public: sal_uInt64 GetDictionaryLength(); PDFDictionaryElement* GetDictionary() const; void SetDictionary(PDFDictionaryElement* pDictionaryElement); + void SetArray(PDFArrayElement* pArrayElement); + PDFArrayElement* GetArray() const; }; /// Dictionary object: a set key-value pairs. @@ -628,6 +633,12 @@ bool PDFDocument::Tokenize(SvStream& rStream, bool bPartial) bool bInXRef = false; // The next number will be an xref offset. bool bInStartXRef = false; + // Last seen object token. + PDFObjectElement* pObject = nullptr; + // Dictionary depth, so we know when we're outside any dictionaries. + int nDictionaryDepth = 0; + // Last seen array token that's outside any dictionaries. + PDFArrayElement* pArray = nullptr; while (true) { char ch; @@ -657,7 +668,10 @@ bool PDFDocument::Tokenize(SvStream& rStream, bool bPartial) rStream.ReadChar(ch); rStream.SeekRel(-2); if (ch == '<') + { m_aElements.push_back(std::unique_ptr(new PDFDictionaryElement())); + ++nDictionaryDepth; + } else m_aElements.push_back(std::unique_ptr(new PDFHexStringElement())); if (!m_aElements.back()->Read(rStream)) @@ -667,6 +681,7 @@ bool PDFDocument::Tokenize(SvStream& rStream, bool bPartial) case '>': { m_aElements.push_back(std::unique_ptr(new PDFEndDictionaryElement())); + --nDictionaryDepth; rStream.SeekRel(-1); if (!m_aElements.back()->Read(rStream)) return false; @@ -674,7 +689,15 @@ bool PDFDocument::Tokenize(SvStream& rStream, bool bPartial) } case '[': { - m_aElements.push_back(std::unique_ptr(new PDFArrayElement())); + auto pArr = new PDFArrayElement(); + m_aElements.push_back(std::unique_ptr(pArr)); + if (nDictionaryDepth == 0) + { + // The array is attached directly, inform the object. + pArray = pArr; + if (pObject) + pObject->SetArray(pArray); + } rStream.SeekRel(-1); if (!m_aElements.back()->Read(rStream)) return false; @@ -683,6 +706,7 @@ bool PDFDocument::Tokenize(SvStream& rStream, bool bPartial) case ']': { m_aElements.push_back(std::unique_ptr(new PDFEndArrayElement())); + pArray = nullptr; rStream.SeekRel(-1); if (!m_aElements.back()->Read(rStream)) return false; @@ -745,9 +769,17 @@ bool PDFDocument::Tokenize(SvStream& rStream, bool bPartial) } if (bObj) - m_aElements.push_back(std::unique_ptr(new PDFObjectElement(*this, pObjectNumber->GetValue(), pGenerationNumber->GetValue()))); + { + pObject = new PDFObjectElement(*this, pObjectNumber->GetValue(), pGenerationNumber->GetValue()); + m_aElements.push_back(std::unique_ptr(pObject)); + } else + { m_aElements.push_back(std::unique_ptr(new PDFReferenceElement(*this, pObjectNumber->GetValue(), pGenerationNumber->GetValue()))); + if (pArray) + // Reference is part of a direct (non-dictionary) array, inform the array. + pArray->PushBack(m_aElements.back().get()); + } if (!m_aElements.back()->Read(rStream)) return false; } @@ -922,14 +954,14 @@ size_t PDFDocument::FindStartXRef(SvStream& rStream) if (nSize != aBuf.size()) aBuf.resize(nSize); OString aPrefix("startxref"); - char* pOffset = strstr(aBuf.data(), aPrefix.getStr()); - if (!pOffset) + auto it = std::search(aBuf.begin(), aBuf.end(), aPrefix.getStr(), aPrefix.getStr() + aPrefix.getLength()); + if (it == aBuf.end()) { SAL_WARN("xmlsecurity.pdfio", "PDFDocument::FindStartXRef: found no startxref"); return 0; } - rStream.SeekRel(pOffset - aBuf.data() + aPrefix.getLength()); + rStream.SeekRel(it - aBuf.begin() + aPrefix.getLength()); if (rStream.IsEof()) { SAL_WARN("xmlsecurity.pdfio", "PDFDocument::FindStartXRef: unexpected end of stream after startxref"); @@ -1009,9 +1041,14 @@ void PDFDocument::ReadXRef(SvStream& rStream) SAL_WARN("xmlsecurity.pdfio", "PDFDocument::ReadXRef: unexpected keyword"); return; } - m_aXRef[nIndex] = aOffset.GetValue(); - // Initially only the first entry is dirty. - m_aXRefDirty[nIndex] = nIndex == 0; + // xrefs are read in reverse order, so never update an existing + // offset with an older one. + if (m_aXRef.find(nIndex) == m_aXRef.end()) + { + m_aXRef[nIndex] = aOffset.GetValue(); + // Initially only the first entry is dirty. + m_aXRefDirty[nIndex] = nIndex == 0; + } PDFDocument::SkipWhitespace(rStream); } } @@ -1133,7 +1170,22 @@ std::vector PDFDocument::GetSignatureWidgets() for (const auto& pPage : aPages) { - auto pAnnots = dynamic_cast(pPage->Lookup("Annots")); + PDFElement* pAnnotsElement = pPage->Lookup("Annots"); + auto pAnnots = dynamic_cast(pAnnotsElement); + if (!pAnnots) + { + // Annots is not an array, see if it's a reference to an object + // with a direct array. + auto pAnnotsRef = dynamic_cast(pAnnotsElement); + if (pAnnotsRef) + { + if (PDFObjectElement* pAnnotsObject = pAnnotsRef->LookupObject()) + { + pAnnots = pAnnotsObject->GetArray(); + } + } + } + if (!pAnnots) continue; @@ -1438,12 +1490,8 @@ bool PDFDocument::ValidateSignature(SvStream& rStream, PDFObjectElement* pSignat } PRTime nSigningTime; - if (NSS_CMSSignerInfo_GetSigningTime(pCMSSignerInfo, &nSigningTime) != SECSuccess) - { - SAL_WARN("xmlsecurity.pdfio", "PDFDocument::ValidateSignature: NSS_CMSSignerInfo_GetSigningTime() failed"); - return false; - } - else + // This may fail, in which case the date should be taken from the dictionary's "M" key. + if (NSS_CMSSignerInfo_GetSigningTime(pCMSSignerInfo, &nSigningTime) == SECSuccess) { // First convert the UNIX timestamp to an ISO8601 string. OUStringBuffer aBuffer; @@ -1679,7 +1727,8 @@ PDFObjectElement::PDFObjectElement(PDFDocument& rDoc, double fObjectValue, doubl m_fGenerationValue(fGenerationValue), m_nDictionaryOffset(0), m_nDictionaryLength(0), - m_pDictionaryElement(nullptr) + m_pDictionaryElement(nullptr), + m_pArrayElement(nullptr) { } @@ -2000,6 +2049,16 @@ void PDFObjectElement::SetDictionary(PDFDictionaryElement* pDictionaryElement) m_pDictionaryElement = pDictionaryElement; } +void PDFObjectElement::SetArray(PDFArrayElement* pArrayElement) +{ + m_pArrayElement = pArrayElement; +} + +PDFArrayElement* PDFObjectElement::GetArray() const +{ + return m_pArrayElement; +} + PDFReferenceElement::PDFReferenceElement(PDFDocument& rDoc, int fObjectValue, int fGenerationValue) : m_rDoc(rDoc), m_fObjectValue(fObjectValue), @@ -2203,7 +2262,7 @@ bool PDFNameElement::Read(SvStream& rStream) rStream.ReadChar(ch); while (!rStream.IsEof()) { - if (isspace(ch) || ch == '/' || ch == '[' || ch == '<' || ch == '(') + if (isspace(ch) || ch == '/' || ch == '[' || ch == '<' || ch == '>' || ch == '(') { rStream.SeekRel(-1); m_aValue = aBuf.makeStringAndClear(); -- cgit