Do more CPDF_SecurityHandler cleanup.
- Use size_t for key length in more places.
- Fix non-const ref parameters.
- Initialize members in the header.
Change-Id: I89e78a140d621b20ed0e9199f44c40b0561b0b26
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/63171
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 e709181..d0dee15 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,
const ByteString& password,
uint8_t* key,
- int keylen,
+ size_t keylen,
bool bIgnoreMeta,
const ByteString& fileId) {
int revision = pEncrypt->GetIntegerFor("R");
@@ -57,9 +57,7 @@
}
uint8_t digest[16];
CRYPT_MD5Finish(&md5, digest);
- uint32_t copy_len = keylen;
- if (copy_len > sizeof(digest))
- copy_len = sizeof(digest);
+ size_t copy_len = std::min(keylen, sizeof(digest));
if (revision >= 3) {
for (int i = 0; i < 50; i++)
CRYPT_MD5Generate(digest, copy_len, digest);
@@ -68,7 +66,7 @@
memcpy(key, digest, copy_len);
}
-bool IsValidKeyLengthForCipher(int cipher, int keylen) {
+bool IsValidKeyLengthForCipher(int cipher, size_t keylen) {
switch (cipher) {
case FXCIPHER_AES:
return keylen == 16 || keylen == 24 || keylen == 32;
@@ -86,15 +84,9 @@
} // namespace
-CPDF_SecurityHandler::CPDF_SecurityHandler()
- : m_Version(0),
- m_Revision(0),
- m_Permissions(0),
- m_Cipher(FXCIPHER_NONE),
- m_KeyLen(0),
- m_bOwnerUnlocked(false) {}
+CPDF_SecurityHandler::CPDF_SecurityHandler() = default;
-CPDF_SecurityHandler::~CPDF_SecurityHandler() {}
+CPDF_SecurityHandler::~CPDF_SecurityHandler() = default;
bool CPDF_SecurityHandler::OnInit(const CPDF_Dictionary* pEncryptDict,
const CPDF_Array* pIdArray,
@@ -134,18 +126,19 @@
static bool LoadCryptInfo(const CPDF_Dictionary* pEncryptDict,
const ByteString& name,
- int& cipher,
- int& keylen) {
+ int* cipher,
+ size_t* keylen_out) {
int Version = pEncryptDict->GetIntegerFor("V");
- cipher = FXCIPHER_RC4;
- keylen = 0;
+ *cipher = FXCIPHER_RC4;
+ *keylen_out = 0;
+ int keylen = 0;
if (Version >= 4) {
const CPDF_Dictionary* pCryptFilters = pEncryptDict->GetDictFor("CF");
if (!pCryptFilters)
return false;
if (name == "Identity") {
- cipher = FXCIPHER_NONE;
+ *cipher = FXCIPHER_NONE;
} else {
const CPDF_Dictionary* pDefFilter = pCryptFilters->GetDictFor(name);
if (!pDefFilter)
@@ -168,17 +161,20 @@
}
keylen = nKeyBits / 8;
ByteString cipher_name = pDefFilter->GetStringFor("CFM");
- if (cipher_name == "AESV2" || cipher_name == "AESV3") {
- cipher = FXCIPHER_AES;
- }
+ if (cipher_name == "AESV2" || cipher_name == "AESV3")
+ *cipher = FXCIPHER_AES;
}
} else {
keylen = Version > 1 ? pEncryptDict->GetIntegerFor("Length", 40) / 8 : 5;
}
- if (keylen > 32 || keylen < 0) {
+
+ if (keylen < 0 || keylen > 32)
return false;
- }
- return IsValidKeyLengthForCipher(cipher, keylen);
+ if (!IsValidKeyLengthForCipher(*cipher, keylen))
+ return false;
+
+ *keylen_out = keylen;
+ return true;
}
bool CPDF_SecurityHandler::LoadDict(const CPDF_Dictionary* pEncryptDict) {
@@ -187,19 +183,19 @@
m_Revision = pEncryptDict->GetIntegerFor("R");
m_Permissions = pEncryptDict->GetIntegerFor("P", -1);
if (m_Version < 4)
- return LoadCryptInfo(pEncryptDict, ByteString(), m_Cipher, m_KeyLen);
+ return LoadCryptInfo(pEncryptDict, ByteString(), &m_Cipher, &m_KeyLen);
ByteString stmf_name = pEncryptDict->GetStringFor("StmF");
ByteString strf_name = pEncryptDict->GetStringFor("StrF");
if (stmf_name != strf_name)
return false;
- return LoadCryptInfo(pEncryptDict, strf_name, m_Cipher, m_KeyLen);
+ return LoadCryptInfo(pEncryptDict, strf_name, &m_Cipher, &m_KeyLen);
}
bool CPDF_SecurityHandler::LoadDict(const CPDF_Dictionary* pEncryptDict,
- int& cipher,
- int& key_len) {
+ int* cipher,
+ size_t* key_len) {
m_pEncryptDict.Reset(pEncryptDict);
m_Version = pEncryptDict->GetIntegerFor("V");
m_Revision = pEncryptDict->GetIntegerFor("R");
@@ -216,8 +212,8 @@
if (!LoadCryptInfo(pEncryptDict, strf_name, cipher, key_len))
return false;
- m_Cipher = cipher;
- m_KeyLen = key_len;
+ m_Cipher = *cipher;
+ m_KeyLen = *key_len;
return true;
}
@@ -446,7 +442,7 @@
memcpy(test, ukey.c_str(), copy_len);
for (int32_t i = 19; i >= 0; i--) {
- for (int j = 0; j < m_KeyLen; j++)
+ for (size_t j = 0; j < m_KeyLen; j++)
tmpkey[j] = m_EncryptKey[j] ^ static_cast<uint8_t>(i);
CRYPT_ArcFourCryptBlock(test, 32, tmpkey, m_KeyLen);
}
@@ -477,9 +473,7 @@
}
}
uint8_t enckey[32] = {};
- uint32_t copy_len = m_KeyLen;
- if (copy_len > sizeof(digest))
- copy_len = sizeof(digest);
+ 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);
@@ -490,7 +484,7 @@
} else {
for (int32_t i = 19; i >= 0; i--) {
uint8_t tempkey[32] = {};
- for (int j = 0; j < m_KeyLen; j++)
+ for (size_t j = 0; j < m_KeyLen; j++)
tempkey[j] = enckey[j] ^ static_cast<uint8_t>(i);
CRYPT_ArcFourCryptBlock(okeybuf, okeylen, tempkey, m_KeyLen);
}
@@ -519,9 +513,9 @@
bool bDefault) {
ASSERT(pEncryptDict);
- int cipher = 0;
- int key_len = 0;
- if (!LoadDict(pEncryptDict, cipher, key_len)) {
+ int cipher = FXCIPHER_NONE;
+ size_t key_len = 0;
+ if (!LoadDict(pEncryptDict, &cipher, &key_len)) {
return;
}
ByteString owner_password_copy = owner_password;
@@ -569,7 +563,7 @@
uint8_t tempkey[32];
if (m_Revision >= 3) {
for (uint8_t i = 1; i <= 19; i++) {
- for (int j = 0; j < key_len; j++)
+ for (size_t j = 0; j < key_len; j++)
tempkey[j] = enckey[j] ^ i;
CRYPT_ArcFourCryptBlock(passcode, 32, tempkey, key_len);
}
@@ -600,7 +594,7 @@
CRYPT_ArcFourCryptBlock(digest, 16, m_EncryptKey, key_len);
uint8_t tempkey[32];
for (uint8_t i = 1; i <= 19; i++) {
- for (int j = 0; j < key_len; j++)
+ for (size_t j = 0; j < key_len; j++)
tempkey[j] = m_EncryptKey[j] ^ i;
CRYPT_ArcFourCryptBlock(digest, 16, tempkey, key_len);
}
diff --git a/core/fpdfapi/parser/cpdf_security_handler.h b/core/fpdfapi/parser/cpdf_security_handler.h
index 8fd661e..0690e6f 100644
--- a/core/fpdfapi/parser/cpdf_security_handler.h
+++ b/core/fpdfapi/parser/cpdf_security_handler.h
@@ -52,8 +52,8 @@
bool LoadDict(const CPDF_Dictionary* pEncryptDict);
bool LoadDict(const CPDF_Dictionary* pEncryptDict,
- int& cipher,
- int& key_len);
+ int* cipher,
+ size_t* key_len);
ByteString GetUserPassword(const ByteString& owner_password) const;
bool CheckPassword(const ByteString& user_password, bool bOwner);
@@ -78,13 +78,13 @@
void InitCryptoHandler();
- int m_Version;
- int m_Revision;
+ bool m_bOwnerUnlocked = false;
+ int m_Version = 0;
+ int m_Revision = 0;
+ uint32_t m_Permissions = 0;
+ int m_Cipher = FXCIPHER_NONE;
+ size_t m_KeyLen = 0;
ByteString m_FileId;
- uint32_t m_Permissions;
- int m_Cipher;
- int m_KeyLen;
- bool m_bOwnerUnlocked;
RetainPtr<const CPDF_Dictionary> m_pEncryptDict;
std::unique_ptr<CPDF_CryptoHandler> m_pCryptoHandler;
uint8_t m_EncryptKey[32];
