Fix MSAN errors in CPDF_SecurityHandler.
Use FX_Random_GenerateMT() where random data is needed, instead of
relying on uninitialized memory and other means to provide random data.
Re-enable portions of CPDFSecurityHandlerEmbedderTests that were
disabled for MSAN. Fix some nits along the way.
Bug: chromium:1032090
Change-Id: I8d870da2f0677d5af789332dfde7fe8bf8d26465
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/63932
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fpdfapi/parser/cpdf_security_handler.cpp b/core/fpdfapi/parser/cpdf_security_handler.cpp
index 0c8eadd..0c98bfb 100644
--- a/core/fpdfapi/parser/cpdf_security_handler.cpp
+++ b/core/fpdfapi/parser/cpdf_security_handler.cpp
@@ -18,7 +18,7 @@
#include "core/fpdfapi/parser/cpdf_dictionary.h"
#include "core/fpdfapi/parser/cpdf_object.h"
#include "core/fpdfapi/parser/cpdf_string.h"
-#include "core/fxcrt/fx_extension.h"
+#include "core/fxcrt/fx_random.h"
#include "third_party/base/ptr_util.h"
namespace {
@@ -521,17 +521,17 @@
ByteString CPDF_SecurityHandler::GetEncodedPassword(
ByteStringView password) const {
switch (m_PasswordEncodingConversion) {
- case CPDF_SecurityHandler::kNone:
+ case kNone:
// Do nothing.
return ByteString(password);
- case CPDF_SecurityHandler::kLatin1ToUtf8:
+ case kLatin1ToUtf8:
return WideString::FromLatin1(password).ToUTF8();
- case CPDF_SecurityHandler::kUtf8toLatin1:
+ case kUtf8toLatin1:
return WideString::FromUTF8(password).ToLatin1();
default:
NOTREACHED();
return ByteString(password);
- };
+ }
}
void CPDF_SecurityHandler::OnCreateInternal(CPDF_Dictionary* pEncryptDict,
@@ -551,12 +551,12 @@
owner_password_copy = user_password;
if (m_Revision >= 5) {
- int t = static_cast<int>(FXSYS_time(nullptr));
+ uint32_t random[4];
+ FX_Random_GenerateMT(random, FX_ArraySize(random));
CRYPT_sha2_context sha;
CRYPT_SHA256Start(&sha);
- CRYPT_SHA256Update(&sha, (uint8_t*)&t, sizeof t);
- CRYPT_SHA256Update(&sha, m_EncryptKey, 32);
- CRYPT_SHA256Update(&sha, (uint8_t*)"there", 5);
+ CRYPT_SHA256Update(&sha, reinterpret_cast<uint8_t*>(random),
+ sizeof(random));
CRYPT_SHA256Finish(&sha, m_EncryptKey);
AES256_SetPassword(pEncryptDict, user_password, false, m_EncryptKey);
if (bDefault)
@@ -716,6 +716,11 @@
buf[10] = 'd';
buf[11] = 'b';
+ // In ISO 32000 Supplement for ExtensionLevel 3, Algorithm 3.10 says bytes 12
+ // to 15 should be random data.
+ uint32_t* buf_random = reinterpret_cast<uint32_t*>(&buf[12]);
+ FX_Random_GenerateMT(buf_random, 1);
+
CRYPT_aes_context aes;
memset(&aes, 0, sizeof(aes));
CRYPT_AESSetKey(&aes, key, 32, true);
diff --git a/core/fpdfapi/parser/cpdf_security_handler_embeddertest.cpp b/core/fpdfapi/parser/cpdf_security_handler_embeddertest.cpp
index a9698a0..e93ca38 100644
--- a/core/fpdfapi/parser/cpdf_security_handler_embeddertest.cpp
+++ b/core/fpdfapi/parser/cpdf_security_handler_embeddertest.cpp
@@ -279,9 +279,6 @@
VerifySavedHelloWorldDocumentWithPassword(kHotelLatin1);
VerifySavedHelloWorldDocumentWithPassword(kHotelUTF8);
-#if !defined(MEMORY_SANITIZER)
- // TODO(crbug.com/1032090): This triggers an MSAN error.
- // Fix and enable this code.
ClearString();
RemoveTrailerIdFromDocument();
EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
@@ -292,7 +289,6 @@
// TODO(thestig): Check and see if this regeneration is necessary.
VerifySavedHelloWorldDocumentWithPassword(kAgeLatin1);
VerifySavedHelloWorldDocumentWithPassword(kAgeUTF8);
-#endif
}
TEST_F(CPDFSecurityHandlerEmbedderTest, OwnerPasswordVersion5Latin1) {
@@ -306,15 +302,11 @@
VerifySavedHelloWorldDocumentWithPassword(kHotelLatin1);
VerifySavedHelloWorldDocumentWithPassword(kHotelUTF8);
-#if !defined(MEMORY_SANITIZER)
- // TODO(crbug.com/1032090): This triggers an MSAN error.
- // Fix and enable this code.
ClearString();
RemoveTrailerIdFromDocument();
EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
VerifySavedHelloWorldDocumentWithPassword(kAgeLatin1);
VerifySavedHelloWorldDocumentWithPassword(kAgeUTF8);
-#endif
}
TEST_F(CPDFSecurityHandlerEmbedderTest, OwnerPasswordVersion6UTF8) {
@@ -328,15 +320,11 @@
VerifySavedHelloWorldDocumentWithPassword(kHotelLatin1);
VerifySavedHelloWorldDocumentWithPassword(kHotelUTF8);
-#if !defined(MEMORY_SANITIZER)
- // TODO(crbug.com/1032090): This triggers an MSAN error.
- // Fix and enable this code.
ClearString();
RemoveTrailerIdFromDocument();
EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
VerifySavedHelloWorldDocumentWithPassword(kAgeLatin1);
VerifySavedHelloWorldDocumentWithPassword(kAgeUTF8);
-#endif
}
TEST_F(CPDFSecurityHandlerEmbedderTest, OwnerPasswordVersion6Latin1) {
@@ -350,15 +338,11 @@
VerifySavedHelloWorldDocumentWithPassword(kHotelLatin1);
VerifySavedHelloWorldDocumentWithPassword(kHotelUTF8);
-#if !defined(MEMORY_SANITIZER)
- // TODO(crbug.com/1032090): This triggers an MSAN error.
- // Fix and enable this code.
ClearString();
RemoveTrailerIdFromDocument();
EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
VerifySavedHelloWorldDocumentWithPassword(kAgeLatin1);
VerifySavedHelloWorldDocumentWithPassword(kAgeUTF8);
-#endif
}
TEST_F(CPDFSecurityHandlerEmbedderTest, UserPasswordVersion2UTF8) {
@@ -457,9 +441,6 @@
VerifySavedHelloWorldDocumentWithPassword(kHotelLatin1);
VerifySavedHelloWorldDocumentWithPassword(kHotelUTF8);
-#if !defined(MEMORY_SANITIZER)
- // TODO(crbug.com/1032090): This triggers an MSAN error.
- // Fix and enable this code.
ClearString();
RemoveTrailerIdFromDocument();
EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
@@ -469,7 +450,6 @@
// TODO(thestig): Check and see if this regeneration is necessary.
VerifySavedHelloWorldDocumentWithPassword(kHotelLatin1);
VerifySavedHelloWorldDocumentWithPassword(kHotelUTF8);
-#endif
}
TEST_F(CPDFSecurityHandlerEmbedderTest, UserPasswordVersion5Latin1) {
@@ -483,15 +463,11 @@
VerifySavedHelloWorldDocumentWithPassword(kHotelLatin1);
VerifySavedHelloWorldDocumentWithPassword(kHotelUTF8);
-#if !defined(MEMORY_SANITIZER)
- // TODO(crbug.com/1032090): This triggers an MSAN error.
- // Fix and enable this code.
ClearString();
RemoveTrailerIdFromDocument();
EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
VerifySavedHelloWorldDocumentWithPassword(kHotelLatin1);
VerifySavedHelloWorldDocumentWithPassword(kHotelUTF8);
-#endif
}
TEST_F(CPDFSecurityHandlerEmbedderTest, UserPasswordVersion6UTF8) {
@@ -505,15 +481,11 @@
VerifySavedHelloWorldDocumentWithPassword(kHotelLatin1);
VerifySavedHelloWorldDocumentWithPassword(kHotelUTF8);
-#if !defined(MEMORY_SANITIZER)
- // TODO(crbug.com/1032090): This triggers an MSAN error.
- // Fix and enable this code.
ClearString();
RemoveTrailerIdFromDocument();
EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
VerifySavedHelloWorldDocumentWithPassword(kHotelLatin1);
VerifySavedHelloWorldDocumentWithPassword(kHotelUTF8);
-#endif
}
TEST_F(CPDFSecurityHandlerEmbedderTest, UserPasswordVersion6Latin1) {
@@ -527,13 +499,9 @@
VerifySavedHelloWorldDocumentWithPassword(kHotelLatin1);
VerifySavedHelloWorldDocumentWithPassword(kHotelUTF8);
-#if !defined(MEMORY_SANITIZER)
- // TODO(crbug.com/1032090): This triggers an MSAN error.
- // Fix and enable this code.
ClearString();
RemoveTrailerIdFromDocument();
EXPECT_TRUE(FPDF_SaveAsCopy(document(), this, 0));
VerifySavedHelloWorldDocumentWithPassword(kHotelLatin1);
VerifySavedHelloWorldDocumentWithPassword(kHotelUTF8);
-#endif
}
