summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHenrique Nakashima <hnakashima@chromium.org>2017-10-10 16:13:08 -0400
committerChromium commit bot <commit-bot@chromium.org>2017-10-10 20:23:26 +0000
commitfb6165ff8f8ad1d7725f63e509eb7f7543df231e (patch)
tree3231f9790fd9d70d1954e7d7c912072fdb3cf2de
parent1886471c3432dee4d9a9be5678a757dde8717652 (diff)
downloadpdfium-chromium/3237.tar.xz
Fix dangling pointer to ID array in CPDF_SecurityHandler.chromium/3238chromium/3237
This was caused by breaking the reference from CPDF_SecurityHandler to CPDF_Parser in https://pdfium-review.googlesource.com/c/pdfium/+/15290 The reference was replaced with a reference to the ID Array and a copy of the password. The issue is that when parsing PDFs with multiple trailers, the trailer containing the ID array may be replaced and destroyed in CPDF_Parser::TrailerData::SetMainTrailer() after being passed to CPDF_SecurityHandler, which would then have a dangling pointer to it. This CL changes the CPDF_SecurityHandler to hold a copy of the original file ID instead of all the ID Array. Bug: chromium:771479,chromium:772376 Change-Id: Id98100502093d890fc2fe6a3da139f910daf38f4 Reviewed-on: https://pdfium-review.googlesource.com/15910 Commit-Queue: Henrique Nakashima <hnakashima@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: Ryan Harrison <rharrison@chromium.org>
-rw-r--r--core/fpdfapi/parser/cpdf_security_handler.cpp42
-rw-r--r--core/fpdfapi/parser/cpdf_security_handler.h2
2 files changed, 21 insertions, 23 deletions
diff --git a/core/fpdfapi/parser/cpdf_security_handler.cpp b/core/fpdfapi/parser/cpdf_security_handler.cpp
index 26202ec3ee..ca0891a729 100644
--- a/core/fpdfapi/parser/cpdf_security_handler.cpp
+++ b/core/fpdfapi/parser/cpdf_security_handler.cpp
@@ -31,7 +31,7 @@ void CalcEncryptKey(const CPDF_Dictionary* pEncrypt,
uint8_t* key,
int keylen,
bool bIgnoreMeta,
- const CPDF_Array* pIdArray) {
+ const ByteString& fileId) {
int revision = pEncrypt->GetIntegerFor("R");
uint8_t passcode[32];
for (uint32_t i = 0; i < 32; i++) {
@@ -46,10 +46,8 @@ void CalcEncryptKey(const CPDF_Dictionary* pEncrypt,
CRYPT_MD5Update(&md5, (uint8_t*)okey.c_str(), okey.GetLength());
uint32_t perm = pEncrypt->GetIntegerFor("P");
CRYPT_MD5Update(&md5, (uint8_t*)&perm, 4);
- if (pIdArray) {
- ByteString id = pIdArray->GetStringAt(0);
- CRYPT_MD5Update(&md5, (uint8_t*)id.c_str(), id.GetLength());
- }
+ if (!fileId.IsEmpty())
+ CRYPT_MD5Update(&md5, (uint8_t*)fileId.c_str(), fileId.GetLength());
if (!bIgnoreMeta && revision >= 3 &&
!pEncrypt->GetIntegerFor("EncryptMetadata", 1)) {
uint32_t tag = 0xFFFFFFFF;
@@ -58,13 +56,11 @@ void CalcEncryptKey(const CPDF_Dictionary* pEncrypt,
uint8_t digest[16];
CRYPT_MD5Finish(&md5, digest);
uint32_t copy_len = keylen;
- if (copy_len > sizeof(digest)) {
+ if (copy_len > sizeof(digest))
copy_len = sizeof(digest);
- }
if (revision >= 3) {
- for (int i = 0; i < 50; i++) {
+ for (int i = 0; i < 50; i++)
CRYPT_MD5Generate(digest, copy_len, digest);
- }
}
memset(key, 0, keylen);
memcpy(key, digest, copy_len);
@@ -102,13 +98,11 @@ CPDF_SecurityHandler::~CPDF_SecurityHandler() {}
bool CPDF_SecurityHandler::OnInit(const CPDF_Dictionary* pEncryptDict,
const CPDF_Array* pIdArray,
const ByteString& password) {
- m_pIdArray = pIdArray;
- if (!LoadDict(pEncryptDict)) {
+ m_FileId = pIdArray ? pIdArray->GetStringAt(0) : "";
+ if (!LoadDict(pEncryptDict))
return false;
- }
- if (m_Cipher == FXCIPHER_NONE) {
+ if (m_Cipher == FXCIPHER_NONE)
return true;
- }
if (!CheckSecurity(password))
return false;
@@ -420,7 +414,7 @@ bool CPDF_SecurityHandler::CheckUserPassword(const ByteString& password,
uint8_t* key,
int32_t key_len) {
CalcEncryptKey(m_pEncryptDict.Get(), password, key, key_len,
- bIgnoreEncryptMeta, m_pIdArray.Get());
+ bIgnoreEncryptMeta, m_FileId);
ByteString ukey =
m_pEncryptDict ? m_pEncryptDict->GetStringFor("U") : ByteString();
if (ukey.GetLength() < 16) {
@@ -447,9 +441,8 @@ bool CPDF_SecurityHandler::CheckUserPassword(const ByteString& password,
CRYPT_md5_context md5;
CRYPT_MD5Start(&md5);
CRYPT_MD5Update(&md5, defpasscode, 32);
- if (m_pIdArray) {
- ByteString id = m_pIdArray->GetStringAt(0);
- CRYPT_MD5Update(&md5, (uint8_t*)id.c_str(), id.GetLength());
+ if (!m_FileId.IsEmpty()) {
+ CRYPT_MD5Update(&md5, (uint8_t*)m_FileId.c_str(), m_FileId.GetLength());
}
CRYPT_MD5Finish(&md5, ukeybuf);
return memcmp(test, ukeybuf, 16) == 0;
@@ -581,8 +574,14 @@ void CPDF_SecurityHandler::OnCreateInternal(CPDF_Dictionary* pEncryptDict,
}
pEncryptDict->SetNewFor<CPDF_String>("O", ByteString(passcode, 32), false);
}
+
+ ByteString fileId;
+ if (pIdArray) {
+ fileId = pIdArray->GetStringAt(0);
+ }
+
CalcEncryptKey(m_pEncryptDict.Get(), user_password, m_EncryptKey, key_len,
- false, pIdArray);
+ false, fileId);
if (m_Revision < 3) {
uint8_t tempbuf[32];
memcpy(tempbuf, defpasscode, 32);
@@ -592,9 +591,8 @@ void CPDF_SecurityHandler::OnCreateInternal(CPDF_Dictionary* pEncryptDict,
CRYPT_md5_context md5;
CRYPT_MD5Start(&md5);
CRYPT_MD5Update(&md5, defpasscode, 32);
- if (pIdArray) {
- ByteString id = pIdArray->GetStringAt(0);
- CRYPT_MD5Update(&md5, (uint8_t*)id.c_str(), id.GetLength());
+ if (!fileId.IsEmpty()) {
+ CRYPT_MD5Update(&md5, (uint8_t*)fileId.c_str(), fileId.GetLength());
}
uint8_t digest[32];
CRYPT_MD5Finish(&md5, digest);
diff --git a/core/fpdfapi/parser/cpdf_security_handler.h b/core/fpdfapi/parser/cpdf_security_handler.h
index 15be505705..0ae2cfe66c 100644
--- a/core/fpdfapi/parser/cpdf_security_handler.h
+++ b/core/fpdfapi/parser/cpdf_security_handler.h
@@ -95,7 +95,7 @@ class CPDF_SecurityHandler {
int m_Version;
int m_Revision;
UnownedPtr<const CPDF_Dictionary> m_pEncryptDict;
- UnownedPtr<const CPDF_Array> m_pIdArray;
+ ByteString m_FileId;
uint32_t m_Permissions;
int m_Cipher;
uint8_t m_EncryptKey[32];