From 27ddf161579f79510b361d0016ccc7f0cdffdc6d Mon Sep 17 00:00:00 2001 From: thestig Date: Mon, 23 May 2016 15:06:59 -0700 Subject: Distinguish between user and owner passwords. BUG=pdfium:496 Review-Url: https://codereview.chromium.org/2005653002 --- BUILD.gn | 1 + core/fpdfapi/fpdf_parser/cpdf_document.cpp | 13 +++++++-- core/fpdfapi/fpdf_parser/cpdf_parser.cpp | 7 ++--- .../fpdf_parser/cpdf_parser_embeddertest.cpp | 2 +- core/fpdfapi/fpdf_parser/cpdf_security_handler.cpp | 32 ++++++++++----------- core/fpdfapi/fpdf_parser/cpdf_security_handler.h | 1 + .../cpdf_security_handler_embeddertest.cpp | 32 +++++++++++++++++++++ core/fpdfapi/fpdf_parser/include/cpdf_document.h | 2 +- core/fpdfapi/fpdf_parser/include/cpdf_parser.h | 2 +- fpdfsdk/fpdfview.cpp | 9 +++--- pdfium.gyp | 1 + testing/embedder_test.cpp | 3 +- testing/embedder_test.h | 1 + testing/resources/encrypted.pdf | Bin 0 -> 10564 bytes 14 files changed, 74 insertions(+), 32 deletions(-) create mode 100644 core/fpdfapi/fpdf_parser/cpdf_security_handler_embeddertest.cpp create mode 100644 testing/resources/encrypted.pdf diff --git a/BUILD.gn b/BUILD.gn index 1dbfea9997..2fdaa885ba 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -1659,6 +1659,7 @@ test("pdfium_embeddertests") { sources = [ "core/fpdfapi/fpdf_page/fpdf_page_func_embeddertest.cpp", "core/fpdfapi/fpdf_parser/cpdf_parser_embeddertest.cpp", + "core/fpdfapi/fpdf_parser/cpdf_security_handler_embeddertest.cpp", "core/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp", "core/fpdfapi/fpdf_render/fpdf_render_loadimage_embeddertest.cpp", "core/fpdfapi/fpdf_render/fpdf_render_pattern_embeddertest.cpp", diff --git a/core/fpdfapi/fpdf_parser/cpdf_document.cpp b/core/fpdfapi/fpdf_parser/cpdf_document.cpp index 79965a23bb..230b9b0d1b 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_document.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_document.cpp @@ -719,9 +719,16 @@ int CPDF_Document::RetrievePageCount() const { return CountPages(pPages, &visited_pages); } -uint32_t CPDF_Document::GetUserPermissions(FX_BOOL bCheckRevision) const { - return m_pParser ? m_pParser->GetPermissions(bCheckRevision) - : static_cast(-1); +uint32_t CPDF_Document::GetUserPermissions() const { + // https://bugs.chromium.org/p/pdfium/issues/detail?id=499 + if (!m_pParser) { +#ifndef PDF_ENABLE_XFA + return 0; +#else // PDF_ENABLE_XFA + return 0xFFFFFFFF; +#endif + } + return m_pParser->GetPermissions(); } FX_BOOL CPDF_Document::IsFormStream(uint32_t objnum, FX_BOOL& bForm) const { diff --git a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp index acf51de1ea..a6b99e5ddb 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_parser.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_parser.cpp @@ -1479,16 +1479,15 @@ CPDF_Dictionary* CPDF_Parser::LoadTrailerV4() { return pObj.release()->AsDictionary(); } -uint32_t CPDF_Parser::GetPermissions(FX_BOOL bCheckRevision) { +uint32_t CPDF_Parser::GetPermissions() const { if (!m_pSecurityHandler) - return (uint32_t)-1; + return 0xFFFFFFFF; uint32_t dwPermission = m_pSecurityHandler->GetPermissions(); if (m_pEncryptDict && m_pEncryptDict->GetStringBy("Filter") == "Standard") { + // See PDF Reference 1.7, page 123, table 3.20. dwPermission &= 0xFFFFFFFC; dwPermission |= 0xFFFFF0C0; - if (bCheckRevision && m_pEncryptDict->GetIntegerBy("R") == 2) - dwPermission &= 0xFFFFF0FF; } return dwPermission; } diff --git a/core/fpdfapi/fpdf_parser/cpdf_parser_embeddertest.cpp b/core/fpdfapi/fpdf_parser/cpdf_parser_embeddertest.cpp index 042b221554..66b29a91c4 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_parser_embeddertest.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_parser_embeddertest.cpp @@ -32,7 +32,7 @@ TEST_F(CPDFParserEmbeddertest, Bug_544880) { } TEST_F(CPDFParserEmbeddertest, Feature_Linearized_Loading) { - EXPECT_TRUE(OpenDocument("feature_linearized_loading.pdf", true)); + EXPECT_TRUE(OpenDocument("feature_linearized_loading.pdf", nullptr, true)); } TEST_F(CPDFParserEmbeddertest, Bug_325) { diff --git a/core/fpdfapi/fpdf_parser/cpdf_security_handler.cpp b/core/fpdfapi/fpdf_parser/cpdf_security_handler.cpp index d6416fb055..94ef042bc0 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_security_handler.cpp +++ b/core/fpdfapi/fpdf_parser/cpdf_security_handler.cpp @@ -67,15 +67,15 @@ void CalcEncryptKey(CPDF_Dictionary* pEncrypt, } // namespace -CPDF_SecurityHandler::CPDF_SecurityHandler() { - m_Version = 0; - m_Revision = 0; - m_pParser = NULL; - m_pEncryptDict = NULL; - m_Permissions = 0; - m_Cipher = FXCIPHER_NONE; - m_KeyLen = 0; -} +CPDF_SecurityHandler::CPDF_SecurityHandler() + : m_Version(0), + m_Revision(0), + m_pParser(nullptr), + m_pEncryptDict(nullptr), + m_Permissions(0), + m_Cipher(FXCIPHER_NONE), + m_KeyLen(0), + m_bOwnerUnlocked(false) {} CPDF_SecurityHandler::~CPDF_SecurityHandler() {} @@ -94,23 +94,21 @@ FX_BOOL CPDF_SecurityHandler::OnInit(CPDF_Parser* pParser, } return CheckSecurity(m_KeyLen); } + FX_BOOL CPDF_SecurityHandler::CheckSecurity(int32_t key_len) { CFX_ByteString password = m_pParser->GetPassword(); - if (CheckPassword(password.raw_str(), password.GetLength(), TRUE, + if (!password.IsEmpty() && + CheckPassword(password.raw_str(), password.GetLength(), TRUE, m_EncryptKey, key_len)) { - if (password.IsEmpty()) { - if (!CheckPassword(password.raw_str(), password.GetLength(), FALSE, - m_EncryptKey, key_len)) { - return FALSE; - } - } + m_bOwnerUnlocked = true; return TRUE; } return CheckPassword(password.raw_str(), password.GetLength(), FALSE, m_EncryptKey, key_len); } + uint32_t CPDF_SecurityHandler::GetPermissions() { - return m_Permissions; + return m_bOwnerUnlocked ? 0xFFFFFFFF : m_Permissions; } static FX_BOOL LoadCryptInfo(CPDF_Dictionary* pEncryptDict, diff --git a/core/fpdfapi/fpdf_parser/cpdf_security_handler.h b/core/fpdfapi/fpdf_parser/cpdf_security_handler.h index 645c97698a..d342c1484e 100644 --- a/core/fpdfapi/fpdf_parser/cpdf_security_handler.h +++ b/core/fpdfapi/fpdf_parser/cpdf_security_handler.h @@ -104,6 +104,7 @@ class CPDF_SecurityHandler { int m_Cipher; uint8_t m_EncryptKey[32]; int m_KeyLen; + bool m_bOwnerUnlocked; }; #endif // CORE_FPDFAPI_FPDF_PARSER_CPDF_SECURITY_HANDLER_H_ diff --git a/core/fpdfapi/fpdf_parser/cpdf_security_handler_embeddertest.cpp b/core/fpdfapi/fpdf_parser/cpdf_security_handler_embeddertest.cpp new file mode 100644 index 0000000000..37b6d8fc33 --- /dev/null +++ b/core/fpdfapi/fpdf_parser/cpdf_security_handler_embeddertest.cpp @@ -0,0 +1,32 @@ +// Copyright 2016 PDFium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "testing/embedder_test.h" +#include "testing/gtest/include/gtest/gtest.h" + +class CPDFSecurityHandlerEmbeddertest : public EmbedderTest {}; + +TEST_F(CPDFSecurityHandlerEmbeddertest, Unencrypted) { + ASSERT_TRUE(OpenDocument("about_blank.pdf")); + EXPECT_EQ(0xFFFFFFFF, FPDF_GetDocPermissions(document())); +} + +TEST_F(CPDFSecurityHandlerEmbeddertest, UnencryptedWithPassword) { + ASSERT_TRUE(OpenDocument("about_blank.pdf", "foobar")); + EXPECT_EQ(0xFFFFFFFF, FPDF_GetDocPermissions(document())); +} + +TEST_F(CPDFSecurityHandlerEmbeddertest, NoPassword) { + EXPECT_FALSE(OpenDocument("encrypted.pdf")); +} + +TEST_F(CPDFSecurityHandlerEmbeddertest, UserPassword) { + ASSERT_TRUE(OpenDocument("encrypted.pdf", "1234")); + EXPECT_EQ(0xFFFFF2C0, FPDF_GetDocPermissions(document())); +} + +TEST_F(CPDFSecurityHandlerEmbeddertest, OwnerPassword) { + ASSERT_TRUE(OpenDocument("encrypted.pdf", "5678")); + EXPECT_EQ(0xFFFFFFFC, FPDF_GetDocPermissions(document())); +} diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_document.h b/core/fpdfapi/fpdf_parser/include/cpdf_document.h index 687619b763..f7a7f9f183 100644 --- a/core/fpdfapi/fpdf_parser/include/cpdf_document.h +++ b/core/fpdfapi/fpdf_parser/include/cpdf_document.h @@ -52,7 +52,7 @@ class CPDF_Document : public CFX_PrivateData, public CPDF_IndirectObjectHolder { int GetPageCount() const; CPDF_Dictionary* GetPage(int iPage); int GetPageIndex(uint32_t objnum); - uint32_t GetUserPermissions(FX_BOOL bCheckRevision = FALSE) const; + uint32_t GetUserPermissions() const; CPDF_DocPageData* GetPageData() const { return m_pDocPage; } void ClearPageData(); void RemoveColorSpaceFromPageData(CPDF_Object* pObject); diff --git a/core/fpdfapi/fpdf_parser/include/cpdf_parser.h b/core/fpdfapi/fpdf_parser/include/cpdf_parser.h index 48511e9263..b468bead9d 100644 --- a/core/fpdfapi/fpdf_parser/include/cpdf_parser.h +++ b/core/fpdfapi/fpdf_parser/include/cpdf_parser.h @@ -38,7 +38,7 @@ class CPDF_Parser { ~CPDF_Parser(); Error StartParse(IFX_FileRead* pFile); - uint32_t GetPermissions(FX_BOOL bCheckRevision = FALSE); + uint32_t GetPermissions() const; void SetPassword(const FX_CHAR* password) { m_Password = password; } CFX_ByteString GetPassword() { return m_Password; } diff --git a/fpdfsdk/fpdfview.cpp b/fpdfsdk/fpdfview.cpp index 020ff8df6d..e7b3a96b0b 100644 --- a/fpdfsdk/fpdfview.cpp +++ b/fpdfsdk/fpdfview.cpp @@ -464,15 +464,16 @@ DLLEXPORT FPDF_BOOL STDCALL FPDF_GetFileVersion(FPDF_DOCUMENT doc, // header). DLLEXPORT unsigned long STDCALL FPDF_GetDocPermissions(FPDF_DOCUMENT document) { CPDF_Document* pDoc = CPDFDocumentFromFPDFDocument(document); - if (!pDoc || !pDoc->GetParser()) + // https://bugs.chromium.org/p/pdfium/issues/detail?id=499 + if (!pDoc) { #ifndef PDF_ENABLE_XFA return 0; #else // PDF_ENABLE_XFA - return (uint32_t)-1; + return 0xFFFFFFFF; #endif // PDF_ENABLE_XFA + } - CPDF_Dictionary* pDict = pDoc->GetParser()->GetEncryptDict(); - return pDict ? pDict->GetIntegerBy("P") : (uint32_t)-1; + return pDoc->GetUserPermissions(); } DLLEXPORT int STDCALL FPDF_GetSecurityHandlerRevision(FPDF_DOCUMENT document) { diff --git a/pdfium.gyp b/pdfium.gyp index 69f6908ccf..3a3bf69f42 100644 --- a/pdfium.gyp +++ b/pdfium.gyp @@ -974,6 +974,7 @@ 'sources': [ 'core/fpdfapi/fpdf_page/fpdf_page_func_embeddertest.cpp', 'core/fpdfapi/fpdf_parser/cpdf_parser_embeddertest.cpp', + 'core/fpdfapi/fpdf_parser/cpdf_security_handler_embeddertest.cpp', 'core/fpdfapi/fpdf_parser/fpdf_parser_decode_embeddertest.cpp', 'core/fpdfapi/fpdf_render/fpdf_render_loadimage_embeddertest.cpp', 'core/fpdfapi/fpdf_render/fpdf_render_pattern_embeddertest.cpp', diff --git a/testing/embedder_test.cpp b/testing/embedder_test.cpp index 38a3dcdeca..033313dbe2 100644 --- a/testing/embedder_test.cpp +++ b/testing/embedder_test.cpp @@ -111,6 +111,7 @@ bool EmbedderTest::CreateEmptyDocument() { } bool EmbedderTest::OpenDocument(const std::string& filename, + const char* password, bool must_linearize) { std::string file_path; if (!PathService::GetTestFilePath(filename, &file_path)) @@ -133,7 +134,7 @@ bool EmbedderTest::OpenDocument(const std::string& filename, avail_ = FPDFAvail_Create(&file_avail_, &file_access_); if (FPDFAvail_IsLinearized(avail_) == PDF_LINEARIZED) { - document_ = FPDFAvail_GetDocument(avail_, nullptr); + document_ = FPDFAvail_GetDocument(avail_, password); if (!document_) { return false; } diff --git a/testing/embedder_test.h b/testing/embedder_test.h index a21a55de2c..a176d944c0 100644 --- a/testing/embedder_test.h +++ b/testing/embedder_test.h @@ -88,6 +88,7 @@ class EmbedderTest : public ::testing::Test, // The filename is relative to the test data directory where we store all the // test files. virtual bool OpenDocument(const std::string& filename, + const char* password = nullptr, bool must_linearize = false); // Perform JavaScript actions that are to run at document open time. diff --git a/testing/resources/encrypted.pdf b/testing/resources/encrypted.pdf new file mode 100644 index 0000000000..b489d56ff2 Binary files /dev/null and b/testing/resources/encrypted.pdf differ -- cgit v1.2.3