From a0f27b61f24b95235b6db843c425fb92cf503e07 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Sun, 14 Jun 2026 10:09:16 +0200 Subject: [PATCH] PDF: throw on reads from an unauthenticated encrypted file Reading an encrypted file without an installed decryptor used to silently serve undecrypted bytes. Guard read_object/read_object_stream so they throw the new UnauthenticatedReadError instead. m_is_encrypted is now set only after the /Encrypt dictionary is resolved during construction, since that read predates any decryptor and would otherwise trip the guard. Adds is_authenticated() and a test that an encrypted file reports as encrypted-but-unauthenticated and throws on parse_document(). Co-Authored-By: Claude Opus 4.8 --- src/odr/exceptions.cpp | 4 ++++ src/odr/exceptions.hpp | 5 +++++ src/odr/internal/pdf/pdf_document_parser.cpp | 22 +++++++++++++++---- src/odr/internal/pdf/pdf_document_parser.hpp | 3 +++ test/src/internal/pdf/pdf_document_parser.cpp | 15 +++++++++++++ 5 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/odr/exceptions.cpp b/src/odr/exceptions.cpp index a382ca202..156bda681 100644 --- a/src/odr/exceptions.cpp +++ b/src/odr/exceptions.cpp @@ -135,4 +135,8 @@ UnsupportedFileEncoding::UnsupportedFileEncoding(const std::string &message) FileEncryptedError::FileEncryptedError() : std::runtime_error("file encrypted error") {} +UnauthenticatedReadError::UnauthenticatedReadError() + : std::runtime_error( + "cannot read encrypted object without authentication") {} + } // namespace odr diff --git a/src/odr/exceptions.hpp b/src/odr/exceptions.hpp index 54704e910..e61563640 100644 --- a/src/odr/exceptions.hpp +++ b/src/odr/exceptions.hpp @@ -230,4 +230,9 @@ struct FileEncryptedError final : std::runtime_error { explicit FileEncryptedError(); }; +/// @brief Read attempted on an encrypted file that has not been authenticated +struct UnauthenticatedReadError final : std::runtime_error { + explicit UnauthenticatedReadError(); +}; + } // namespace odr diff --git a/src/odr/internal/pdf/pdf_document_parser.cpp b/src/odr/internal/pdf/pdf_document_parser.cpp index 794b73524..1cc217c42 100644 --- a/src/odr/internal/pdf/pdf_document_parser.cpp +++ b/src/odr/internal/pdf/pdf_document_parser.cpp @@ -1,5 +1,6 @@ #include +#include #include #include @@ -263,10 +264,7 @@ DocumentParser::DocumentParser(std::unique_ptr in, if (m_trailer.has_key("Encrypt")) { // Build an `Authenticator` from the trailer `/Encrypt` and `/ID` - // (ISO 32000-1 7.6). `m_is_encrypted` records that the file declares - // encryption regardless of whether we can authenticate it, so an - // unsupported handler still reports as encrypted. - m_is_encrypted = true; + // (ISO 32000-1 7.6). // The `/Encrypt` dictionary's own `/O`,`/U`,… strings are never encrypted // (7.6.2), and need no explicit self-skip guard: it is resolved here while @@ -290,6 +288,12 @@ DocumentParser::DocumentParser(std::unique_ptr in, } } + // Set only after resolving `/Encrypt` above: `read_object` throws on an + // encrypted-but-unauthenticated read, and that resolution runs before any + // decryptor exists. `m_is_encrypted` records that the file declares + // encryption regardless of whether we can authenticate it, so an + // unsupported handler still reports as encrypted. + m_is_encrypted = true; m_authenticator = Authenticator::create(encrypt.as_dictionary(), id0); } @@ -310,6 +314,10 @@ const Dictionary &DocumentParser::trailer() const { return m_trailer; } bool DocumentParser::is_encrypted() const { return m_is_encrypted; } +bool DocumentParser::is_authenticated() const { + return m_is_encrypted && m_decryptor.has_value(); +} + const std::optional &DocumentParser::authenticator() const { return m_authenticator; } @@ -350,6 +358,9 @@ DocumentParser::read_object(const ObjectReference &reference) { // case here: it is read and cached during construction before the // decryptor is installed, so its un-decrypted /O,/U,… strings are served // from the cache and never reach this path. + if (is_encrypted() && !is_authenticated()) { + throw UnauthenticatedReadError(); + } if (m_decryptor.has_value()) { decrypt_strings(object.object, object.reference); } @@ -422,6 +433,9 @@ std::string DocumentParser::read_object_stream(const IndirectObject &object) { // during the trailer-chain walk, before the decryptor exists, so they are // never decrypted (7.5.8.2); object streams are decrypted here as a whole, // leaving their members' plaintext. + if (is_encrypted() && !is_authenticated()) { + throw UnauthenticatedReadError(); + } if (m_decryptor.has_value()) { raw = m_decryptor->decrypt_stream(object.reference, std::move(raw)); } diff --git a/src/odr/internal/pdf/pdf_document_parser.hpp b/src/odr/internal/pdf/pdf_document_parser.hpp index db54d91a0..ffd60556e 100644 --- a/src/odr/internal/pdf/pdf_document_parser.hpp +++ b/src/odr/internal/pdf/pdf_document_parser.hpp @@ -61,6 +61,9 @@ class DocumentParser { /// Whether the file declares an `/Encrypt` dictionary. [[nodiscard]] bool is_encrypted() const; + /// Whether the file is encrypted and a decryptor is installed, so reads can + /// decrypt. False for an encrypted file that has not been unlocked. + [[nodiscard]] bool is_authenticated() const; /// The authenticator for an encrypted file (validates passwords and produces /// a `Decryptor`), or `nullopt` if the file is not encrypted. [[nodiscard]] const std::optional &authenticator() const; diff --git a/test/src/internal/pdf/pdf_document_parser.cpp b/test/src/internal/pdf/pdf_document_parser.cpp index f4579c191..327987145 100644 --- a/test/src/internal/pdf/pdf_document_parser.cpp +++ b/test/src/internal/pdf/pdf_document_parser.cpp @@ -1,3 +1,5 @@ +#include + #include #include #include @@ -147,6 +149,19 @@ TEST(DocumentParser, reopen_with_decryptor) { } } +// Reading an encrypted file without authenticating must throw rather than serve +// undecrypted bytes. The file reports as encrypted but not authenticated. +TEST(DocumentParser, read_without_authentication_throws) { + const auto file = std::make_shared( + TestData::test_file_path("odr-public/pdf/Casio_WVA-M650-7AJF.pdf")); + + DocumentParser parser(file->stream()); + EXPECT_TRUE(parser.is_encrypted()); + EXPECT_FALSE(parser.is_authenticated()); + + EXPECT_THROW((void)parser.parse_document(), odr::UnauthenticatedReadError); +} + TEST(DocumentParser, inherited_page_attributes) { PdfFileBuilder builder; builder.object("<< /Type /Catalog /Pages 2 0 R >>")