Mark UNSAFE_BUFFERS in cpdf_{crypto,security}_handler.cpp
Mark large sections of code without any attempt at re-writing
Bug: pdfium:2154
Change-Id: I5edad8429920cc4b3b07e0bc415e2dd52bbafe26
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/119210
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_crypto_handler.cpp b/core/fpdfapi/parser/cpdf_crypto_handler.cpp
index 7851f3a..7ce9efa 100644
--- a/core/fpdfapi/parser/cpdf_crypto_handler.cpp
+++ b/core/fpdfapi/parser/cpdf_crypto_handler.cpp
@@ -4,11 +4,6 @@
// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-#if defined(UNSAFE_BUFFERS_BUILD)
-// TODO(crbug.com/pdfium/2153): resolve buffer safety issues.
-#pragma allow_unsafe_buffers
-#endif
-
#include "core/fpdfapi/parser/cpdf_crypto_handler.h"
#include <time.h>
@@ -57,51 +52,54 @@
pdfium::span<const uint8_t> source,
uint8_t* dest_buf,
size_t& dest_size) const {
- if (m_Cipher == Cipher::kNone) {
- FXSYS_memcpy(dest_buf, source.data(), source.size());
- return;
- }
- uint8_t realkey[16];
- size_t realkeylen = sizeof(realkey);
- if (m_Cipher != Cipher::kAES || m_KeyLen != 32) {
- uint8_t key1[32];
- PopulateKey(objnum, gennum, key1);
- if (m_Cipher == Cipher::kAES) {
- FXSYS_memcpy(key1 + m_KeyLen + 5, "sAlT", 4);
- }
- size_t len = m_Cipher == Cipher::kAES ? m_KeyLen + 9 : m_KeyLen + 5;
- CRYPT_MD5Generate(pdfium::make_span(key1).first(len), realkey);
- realkeylen = std::min(m_KeyLen + 5, sizeof(realkey));
- }
- if (m_Cipher == Cipher::kAES) {
- CRYPT_AESSetKey(m_pAESContext.get(),
- m_KeyLen == 32 ? m_EncryptKey.data() : realkey, m_KeyLen);
- uint8_t iv[16];
- for (int i = 0; i < 16; i++) {
- iv[i] = (uint8_t)rand();
- }
- CRYPT_AESSetIV(m_pAESContext.get(), iv);
- FXSYS_memcpy(dest_buf, iv, 16);
- int nblocks = source.size() / 16;
- CRYPT_AESEncrypt(m_pAESContext.get(), dest_buf + 16, source.data(),
- nblocks * 16);
- uint8_t padding[16];
- FXSYS_memcpy(padding, source.data() + nblocks * 16, source.size() % 16);
- FXSYS_memset(padding + source.size() % 16, 16 - source.size() % 16,
- 16 - source.size() % 16);
- CRYPT_AESEncrypt(m_pAESContext.get(), dest_buf + nblocks * 16 + 16, padding,
- 16);
- dest_size = 32 + nblocks * 16;
- } else {
- DCHECK_EQ(dest_size, source.size());
- if (dest_buf != source.data()) {
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS({
+ if (m_Cipher == Cipher::kNone) {
FXSYS_memcpy(dest_buf, source.data(), source.size());
+ return;
}
- // SAFETY: caller ensures that dest_buf points to at least dest_size bytes.
- CRYPT_ArcFourCryptBlock(
- UNSAFE_BUFFERS(pdfium::make_span(dest_buf, dest_size)),
- pdfium::make_span(realkey).first(realkeylen));
- }
+ uint8_t realkey[16];
+ size_t realkeylen = sizeof(realkey);
+ if (m_Cipher != Cipher::kAES || m_KeyLen != 32) {
+ uint8_t key1[32];
+ PopulateKey(objnum, gennum, key1);
+ if (m_Cipher == Cipher::kAES) {
+ FXSYS_memcpy(key1 + m_KeyLen + 5, "sAlT", 4);
+ }
+ size_t len = m_Cipher == Cipher::kAES ? m_KeyLen + 9 : m_KeyLen + 5;
+ CRYPT_MD5Generate(pdfium::make_span(key1).first(len), realkey);
+ realkeylen = std::min(m_KeyLen + 5, sizeof(realkey));
+ }
+ if (m_Cipher == Cipher::kAES) {
+ CRYPT_AESSetKey(m_pAESContext.get(),
+ m_KeyLen == 32 ? m_EncryptKey.data() : realkey, m_KeyLen);
+ uint8_t iv[16];
+ for (int i = 0; i < 16; i++) {
+ iv[i] = (uint8_t)rand();
+ }
+ CRYPT_AESSetIV(m_pAESContext.get(), iv);
+ FXSYS_memcpy(dest_buf, iv, 16);
+ int nblocks = source.size() / 16;
+ CRYPT_AESEncrypt(m_pAESContext.get(), dest_buf + 16, source.data(),
+ nblocks * 16);
+ uint8_t padding[16];
+ FXSYS_memcpy(padding, source.data() + nblocks * 16, source.size() % 16);
+ FXSYS_memset(padding + source.size() % 16, 16 - source.size() % 16,
+ 16 - source.size() % 16);
+ CRYPT_AESEncrypt(m_pAESContext.get(), dest_buf + nblocks * 16 + 16,
+ padding, 16);
+ dest_size = 32 + nblocks * 16;
+ } else {
+ DCHECK_EQ(dest_size, source.size());
+ if (dest_buf != source.data()) {
+ FXSYS_memcpy(dest_buf, source.data(), source.size());
+ }
+ // SAFETY: caller ensures that dest_buf points to at least dest_size
+ // bytes.
+ CRYPT_ArcFourCryptBlock(pdfium::make_span(dest_buf, dest_size),
+ pdfium::make_span(realkey).first(realkeylen));
+ }
+ });
}
struct AESCryptContext {
@@ -125,8 +123,10 @@
uint8_t key1[48];
PopulateKey(objnum, gennum, key1);
- if (m_Cipher == Cipher::kAES)
- FXSYS_memcpy(key1 + m_KeyLen + 5, "sAlT", 4);
+ if (m_Cipher == Cipher::kAES) {
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS(FXSYS_memcpy(key1 + m_KeyLen + 5, "sAlT", 4));
+ }
uint8_t realkey[16];
size_t len = m_Cipher == Cipher::kAES ? m_KeyLen + 9 : m_KeyLen + 5;
@@ -171,8 +171,9 @@
if (copy_size > src_left) {
copy_size = src_left;
}
- FXSYS_memcpy(pContext->m_Block + pContext->m_BlockOffset,
- source.data() + src_off, copy_size);
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS(FXSYS_memcpy(pContext->m_Block + pContext->m_BlockOffset,
+ source.data() + src_off, copy_size));
src_off += copy_size;
src_left -= copy_size;
pContext->m_BlockOffset += copy_size;
@@ -333,11 +334,13 @@
DCHECK(cipher != Cipher::kAES2 || keylen == 32);
DCHECK(cipher != Cipher::kRC4 || (keylen >= 5 && keylen <= 16));
- if (m_Cipher != Cipher::kNone)
- FXSYS_memcpy(m_EncryptKey.data(), key, m_KeyLen);
-
- if (m_Cipher == Cipher::kAES)
+ if (m_Cipher != Cipher::kNone) {
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS(FXSYS_memcpy(m_EncryptKey.data(), key, m_KeyLen));
+ }
+ if (m_Cipher == Cipher::kAES) {
m_pAESContext.reset(FX_Alloc(CRYPT_aes_context, 1));
+ }
}
CPDF_CryptoHandler::~CPDF_CryptoHandler() = default;
@@ -345,10 +348,13 @@
void CPDF_CryptoHandler::PopulateKey(uint32_t objnum,
uint32_t gennum,
uint8_t* key) const {
- FXSYS_memcpy(key, m_EncryptKey.data(), m_KeyLen);
- key[m_KeyLen + 0] = (uint8_t)objnum;
- key[m_KeyLen + 1] = (uint8_t)(objnum >> 8);
- key[m_KeyLen + 2] = (uint8_t)(objnum >> 16);
- key[m_KeyLen + 3] = (uint8_t)gennum;
- key[m_KeyLen + 4] = (uint8_t)(gennum >> 8);
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS({
+ FXSYS_memcpy(key, m_EncryptKey.data(), m_KeyLen);
+ key[m_KeyLen + 0] = (uint8_t)objnum;
+ key[m_KeyLen + 1] = (uint8_t)(objnum >> 8);
+ key[m_KeyLen + 2] = (uint8_t)(objnum >> 16);
+ key[m_KeyLen + 3] = (uint8_t)gennum;
+ key[m_KeyLen + 4] = (uint8_t)(gennum >> 8);
+ });
}
diff --git a/core/fpdfapi/parser/cpdf_security_handler.cpp b/core/fpdfapi/parser/cpdf_security_handler.cpp
index c932d63..9145b00 100644
--- a/core/fpdfapi/parser/cpdf_security_handler.cpp
+++ b/core/fpdfapi/parser/cpdf_security_handler.cpp
@@ -4,11 +4,6 @@
// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-#if defined(UNSAFE_BUFFERS_BUILD)
-// TODO(crbug.com/pdfium/2153): resolve buffer safety issues.
-#pragma allow_unsafe_buffers
-#endif
-
#include "core/fpdfapi/parser/cpdf_security_handler.h"
#include <stdint.h>
@@ -44,10 +39,13 @@
DCHECK_EQ(sizeof(kDefaultPasscode), output.size());
size_t len = std::min(password.GetLength(), output.size());
size_t remaining = output.size() - len;
- FXSYS_memcpy(output.data(), password.unsigned_str(), len);
- if (remaining) {
- FXSYS_memcpy(&output[len], kDefaultPasscode, remaining);
- }
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS({
+ FXSYS_memcpy(output.data(), password.unsigned_str(), len);
+ if (remaining) {
+ FXSYS_memcpy(&output[len], kDefaultPasscode, remaining);
+ }
+ });
}
void CalcEncryptKey(const CPDF_Dictionary* pEncrypt,
@@ -80,8 +78,11 @@
CRYPT_MD5Generate(pdfium::make_span(digest).first(copy_len), digest);
}
}
- FXSYS_memset(key, 0, keylen);
- FXSYS_memcpy(key, digest, copy_len);
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS({
+ FXSYS_memset(key, 0, keylen);
+ FXSYS_memcpy(key, digest, copy_len);
+ });
}
bool IsValidKeyLengthForCipher(CPDF_CryptoHandler::Cipher cipher,
@@ -127,7 +128,8 @@
DataVector<uint8_t> inter_digest;
uint8_t* input = digest;
uint8_t* key = input;
- uint8_t* iv = input + 16;
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ uint8_t* iv = UNSAFE_BUFFERS(input + 16);
int i = 0;
size_t block_size = 32;
CRYPT_aes_context aes = {};
@@ -140,12 +142,15 @@
auto encrypted_output_span = pdfium::make_span(encrypted_output);
DataVector<uint8_t> content;
for (int j = 0; j < 64; ++j) {
- content.insert(std::end(content), password.unsigned_str(),
- password.unsigned_str() + password.GetLength());
- content.insert(std::end(content), input, input + block_size);
- if (vector) {
- content.insert(std::end(content), vector, vector + 48);
- }
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS({
+ content.insert(std::end(content), password.unsigned_str(),
+ password.unsigned_str() + password.GetLength());
+ content.insert(std::end(content), input, input + block_size);
+ if (vector) {
+ content.insert(std::end(content), vector, vector + 48);
+ }
+ });
}
CRYPT_AESSetKey(&aes, key, 16);
CRYPT_AESSetIV(&aes, iv);
@@ -179,11 +184,13 @@
encrypted_output_span.size(), input);
}
key = input;
- iv = input + 16;
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ iv = UNSAFE_BUFFERS(input + 16);
++i;
} while (i < 64 || i - 32 < encrypted_output.back());
if (hash) {
- FXSYS_memcpy(hash, input, 32);
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS(FXSYS_memcpy(hash, input, 32));
}
}
@@ -343,12 +350,14 @@
CRYPT_sha2_context sha;
uint8_t digest[32];
if (m_Revision >= 6) {
- Revision6_Hash(password, (const uint8_t*)pkey + 32,
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ Revision6_Hash(password, UNSAFE_BUFFERS((const uint8_t*)pkey + 32),
bOwner ? ukey.unsigned_str() : nullptr, digest);
} else {
CRYPT_SHA256Start(&sha);
CRYPT_SHA256Update(&sha, password.unsigned_str(), password.GetLength());
- CRYPT_SHA256Update(&sha, pkey + 32, 8);
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ CRYPT_SHA256Update(&sha, UNSAFE_BUFFERS(pkey + 32), 8);
if (bOwner)
CRYPT_SHA256Update(&sha, ukey.unsigned_str(), 48);
CRYPT_SHA256Finish(&sha, digest);
@@ -357,12 +366,14 @@
return false;
if (m_Revision >= 6) {
- Revision6_Hash(password, (const uint8_t*)pkey + 40,
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ Revision6_Hash(password, UNSAFE_BUFFERS((const uint8_t*)pkey + 40),
bOwner ? ukey.unsigned_str() : nullptr, digest);
} else {
CRYPT_SHA256Start(&sha);
CRYPT_SHA256Update(&sha, password.unsigned_str(), password.GetLength());
- CRYPT_SHA256Update(&sha, pkey + 40, 8);
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ CRYPT_SHA256Update(&sha, UNSAFE_BUFFERS(pkey + 40), 8);
if (bOwner)
CRYPT_SHA256Update(&sha, ukey.unsigned_str(), 48);
CRYPT_SHA256Finish(&sha, digest);
@@ -385,7 +396,8 @@
uint8_t perms_buf[16] = {};
size_t copy_len =
std::min(sizeof(perms_buf), static_cast<size_t>(perms.GetLength()));
- FXSYS_memcpy(perms_buf, perms.unsigned_str(), copy_len);
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS(FXSYS_memcpy(perms_buf, perms.unsigned_str(), copy_len));
uint8_t buf[16];
CRYPT_AESDecrypt(&aes, buf, perms_buf, 16);
if (buf[9] != 'a' || buf[10] != 'd' || buf[11] != 'b')
@@ -455,7 +467,9 @@
uint8_t ukeybuf[32];
if (m_Revision == 2) {
- FXSYS_memcpy(ukeybuf, kDefaultPasscode, sizeof(kDefaultPasscode));
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS(
+ FXSYS_memcpy(ukeybuf, kDefaultPasscode, sizeof(kDefaultPasscode)));
CRYPT_ArcFourCryptBlock(ukeybuf,
pdfium::make_span(m_EncryptKey).first(m_KeyLen));
return memcmp(ukey.c_str(), ukeybuf, 16) == 0;
@@ -465,10 +479,12 @@
uint8_t tmpkey[32] = {};
uint32_t copy_len = std::min(sizeof(test), ukey.GetLength());
- FXSYS_memcpy(test, ukey.c_str(), copy_len);
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS(FXSYS_memcpy(test, ukey.c_str(), copy_len));
for (int32_t i = 19; i >= 0; i--) {
for (size_t j = 0; j < m_KeyLen; j++) {
- tmpkey[j] = m_EncryptKey[j] ^ static_cast<uint8_t>(i);
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS(tmpkey[j] = m_EncryptKey[j] ^ static_cast<uint8_t>(i));
}
CRYPT_ArcFourCryptBlock(test, pdfium::make_span(tmpkey).first(m_KeyLen));
}
@@ -498,10 +514,13 @@
CRYPT_MD5Generate(digest, digest);
}
uint8_t enckey[32] = {};
- size_t copy_len = std::min(m_KeyLen, sizeof(digest));
- FXSYS_memcpy(enckey, digest, copy_len);
uint8_t okeybuf[32] = {};
- FXSYS_memcpy(okeybuf, okey.c_str(), okeylen);
+ size_t copy_len = std::min(m_KeyLen, sizeof(digest));
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS({
+ FXSYS_memcpy(enckey, digest, copy_len);
+ FXSYS_memcpy(okeybuf, okey.c_str(), okeylen);
+ });
pdfium::span<uint8_t> okey_span = pdfium::make_span(okeybuf).first(okeylen);
if (m_Revision == 2) {
CRYPT_ArcFourCryptBlock(okey_span,
@@ -510,16 +529,20 @@
for (int32_t i = 19; i >= 0; i--) {
uint8_t tempkey[32] = {};
for (size_t j = 0; j < m_KeyLen; j++) {
- tempkey[j] = enckey[j] ^ static_cast<uint8_t>(i);
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS(tempkey[j] = enckey[j] ^ static_cast<uint8_t>(i));
}
CRYPT_ArcFourCryptBlock(okey_span,
pdfium::make_span(tempkey).first(m_KeyLen));
}
}
size_t len = kRequiredOkeyLength;
- while (len && kDefaultPasscode[len - 1] == okey_span[len - 1])
- len--;
-
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS({
+ while (len && kDefaultPasscode[len - 1] == okey_span[len - 1]) {
+ len--;
+ }
+ });
return ByteString(okeybuf, len);
}
@@ -580,7 +603,9 @@
false, file_id);
if (m_Revision < 3) {
uint8_t tempbuf[32];
- FXSYS_memcpy(tempbuf, kDefaultPasscode, sizeof(kDefaultPasscode));
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS(
+ FXSYS_memcpy(tempbuf, kDefaultPasscode, sizeof(kDefaultPasscode)));
CRYPT_ArcFourCryptBlock(tempbuf,
pdfium::make_span(m_EncryptKey).first(key_len));
pEncryptDict->SetNewFor<CPDF_String>("U", ByteString(tempbuf, 32), false);
@@ -598,7 +623,8 @@
uint8_t tempkey[32];
for (uint8_t i = 1; i <= 19; i++) {
for (size_t j = 0; j < key_len; j++) {
- tempkey[j] = m_EncryptKey[j] ^ i;
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS(tempkey[j] = m_EncryptKey[j] ^ i);
}
CRYPT_ArcFourCryptBlock(partial_digest_span,
pdfium::make_span(tempkey).first(key_len));
@@ -631,14 +657,17 @@
CRYPT_SHA256Update(&sha2, digest, 8);
CRYPT_SHA256Finish(&sha2, digest1);
}
- FXSYS_memcpy(digest1 + 32, digest, 16);
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ UNSAFE_BUFFERS(FXSYS_memcpy(digest1 + 32, digest, 16));
pEncryptDict->SetNewFor<CPDF_String>("U", ByteString(digest1, 48), false);
if (m_Revision >= 6) {
- Revision6_Hash(password, digest + 8, nullptr, digest1);
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ Revision6_Hash(password, UNSAFE_BUFFERS(digest + 8), nullptr, digest1);
} else {
CRYPT_SHA256Start(&sha2);
CRYPT_SHA256Update(&sha2, password.unsigned_str(), password.GetLength());
- CRYPT_SHA256Update(&sha2, digest + 8, 8);
+ // TODO(crbug.com/pdfium/2155): investigate safety.
+ CRYPT_SHA256Update(&sha2, UNSAFE_BUFFERS(digest + 8), 8);
CRYPT_SHA256Finish(&sha2, digest1);
}
CRYPT_aes_context aes = {};
