Avoid crash in CPDF_SecurityHandler::GetUserPassword().
For r2 and r3 encrypted PDFs, the value for the /O key is normally a
32-byte string. CPDF_SecurityHandler::GetUserPassword() recently started
using spans, and that caught an OOB error for /O keys shorter than
32-bytes. While a hard crash is better than an OOB read, gracefully
handling the situation is best. Change GetUserPassword() to do that.
For testing, add hand-edited copies of encrypted_hello_world_r2.pdf and
encrypted_hello_world_r3.pdf, with 31-byte /O keys, and the xref offset
adjusted. Then make sure embedder tests that try to load these bad PDFs
do not crash.
Bug: pdfium:1436
Change-Id: I873eead7c1dc45c9c4bbd41044f8fa5b77d7f558
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/64170
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_security_handler.cpp b/core/fpdfapi/parser/cpdf_security_handler.cpp
index d65ce4b..126fc6b 100644
--- a/core/fpdfapi/parser/cpdf_security_handler.cpp
+++ b/core/fpdfapi/parser/cpdf_security_handler.cpp
@@ -469,7 +469,13 @@
ByteString CPDF_SecurityHandler::GetUserPassword(
const ByteString& owner_password) const {
+ constexpr size_t kRequiredOkeyLength = 32;
ByteString okey = m_pEncryptDict->GetStringFor("O");
+ size_t okeylen = std::min<size_t>(okey.GetLength(), kRequiredOkeyLength);
+ if (okeylen < kRequiredOkeyLength)
+ return ByteString();
+
+ DCHECK_EQ(kRequiredOkeyLength, okeylen);
uint8_t passcode[32];
GetPassCode(owner_password, passcode);
uint8_t digest[16];
@@ -482,8 +488,7 @@
size_t copy_len = std::min(m_KeyLen, sizeof(digest));
memcpy(enckey, digest, copy_len);
- size_t okeylen = std::min<size_t>(okey.GetLength(), 32);
- uint8_t okeybuf[64] = {};
+ uint8_t okeybuf[32] = {};
memcpy(okeybuf, okey.c_str(), okeylen);
pdfium::span<uint8_t> okey_span(okeybuf, okeylen);
if (m_Revision == 2) {
@@ -496,7 +501,7 @@
CRYPT_ArcFourCryptBlock(okey_span, {tempkey, m_KeyLen});
}
}
- size_t len = 32;
+ size_t len = kRequiredOkeyLength;
while (len && kDefaultPasscode[len - 1] == okey_span[len - 1])
len--;
diff --git a/core/fpdfapi/parser/cpdf_security_handler_embeddertest.cpp b/core/fpdfapi/parser/cpdf_security_handler_embeddertest.cpp
index a7a0a79..ee2a621 100644
--- a/core/fpdfapi/parser/cpdf_security_handler_embeddertest.cpp
+++ b/core/fpdfapi/parser/cpdf_security_handler_embeddertest.cpp
@@ -222,6 +222,18 @@
EXPECT_EQ(0xFFFFFFFC, FPDF_GetDocPermissions(document()));
}
+// Should not crash. https://crbug.com/pdfium/1436
+TEST_F(CPDFSecurityHandlerEmbedderTest, BadOkeyVersion2) {
+ EXPECT_FALSE(
+ OpenDocumentWithPassword("encrypted_hello_world_r2_bad_okey.pdf", "a"));
+}
+
+// Should not crash. https://crbug.com/pdfium/1436
+TEST_F(CPDFSecurityHandlerEmbedderTest, BadOkeyVersion3) {
+ EXPECT_FALSE(
+ OpenDocumentWithPassword("encrypted_hello_world_r3_bad_okey.pdf", "a"));
+}
+
TEST_F(CPDFSecurityHandlerEmbedderTest, OwnerPasswordVersion2UTF8) {
// The password is "age", where the 'a' has a circumflex. Encoding the
// password as UTF-8 works.
diff --git a/testing/resources/encrypted_hello_world_r2_bad_okey.pdf b/testing/resources/encrypted_hello_world_r2_bad_okey.pdf
new file mode 100644
index 0000000..637812b
--- /dev/null
+++ b/testing/resources/encrypted_hello_world_r2_bad_okey.pdf
Binary files differ
diff --git a/testing/resources/encrypted_hello_world_r3_bad_okey.pdf b/testing/resources/encrypted_hello_world_r3_bad_okey.pdf
new file mode 100644
index 0000000..721c20b
--- /dev/null
+++ b/testing/resources/encrypted_hello_world_r3_bad_okey.pdf
Binary files differ
