Convert fx_random.cpp to use safe types
Simpler to make this conversion than to justify 10 or so uses of
unsafe buffers.
-- beef up test for one caller thru this entire codepath.
Change-Id: If8f5e93ee4f307f28735dcc7129fd496706930a4
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/116554
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_security_handler.cpp b/core/fpdfapi/parser/cpdf_security_handler.cpp
index 175fd9d..fd68d79 100644
--- a/core/fpdfapi/parser/cpdf_security_handler.cpp
+++ b/core/fpdfapi/parser/cpdf_security_handler.cpp
@@ -22,6 +22,7 @@
#include "core/fxcrt/data_vector.h"
#include "core/fxcrt/fx_memcpy_wrappers.h"
#include "core/fxcrt/fx_random.h"
+#include "core/fxcrt/span_util.h"
#include "third_party/base/check.h"
#include "third_party/base/check_op.h"
#include "third_party/base/notreached.h"
@@ -555,7 +556,7 @@
if (m_Revision >= 5) {
uint32_t random[4];
- FX_Random_GenerateMT(random, std::size(random));
+ FX_Random_GenerateMT(random);
CRYPT_sha2_context sha;
CRYPT_SHA256Start(&sha);
CRYPT_SHA256Update(&sha, reinterpret_cast<uint8_t*>(random),
@@ -655,8 +656,8 @@
// 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);
+ auto random_span = pdfium::make_span(buf).subspan(12, 4);
+ FX_Random_GenerateMT(fxcrt::reinterpret_span<uint32_t>(random_span));
CRYPT_aes_context aes = {};
CRYPT_AESSetKey(&aes, m_EncryptKey, sizeof(m_EncryptKey));
diff --git a/core/fxcrt/fx_random.cpp b/core/fxcrt/fx_random.cpp
index 8a8182d..26a0596 100644
--- a/core/fxcrt/fx_random.cpp
+++ b/core/fxcrt/fx_random.cpp
@@ -6,6 +6,8 @@
#include "core/fxcrt/fx_random.h"
+#include <array>
+
#include "build/build_config.h"
#include "core/fxcrt/fx_memory.h"
#include "core/fxcrt/fx_string.h"
@@ -28,7 +30,7 @@
struct MTContext {
uint32_t mti;
- uint32_t mt[MT_N];
+ std::array<uint32_t, MT_N> mt;
};
bool g_bHaveGlobalSeed = false;
@@ -84,35 +86,34 @@
void* FX_Random_MT_Start(uint32_t dwSeed) {
MTContext* pContext = FX_Alloc(MTContext, 1);
- uint32_t* pBuf = pContext->mt;
- pBuf[0] = dwSeed;
- for (uint32_t i = 1; i < MT_N; i++)
- pBuf[i] = (1812433253UL * (pBuf[i - 1] ^ (pBuf[i - 1] >> 30)) + i);
-
+ pContext->mt[0] = dwSeed;
+ for (uint32_t i = 1; i < MT_N; i++) {
+ const uint32_t prev = pContext->mt[i - 1];
+ pContext->mt[i] = (1812433253UL * (prev ^ (prev >> 30)) + i);
+ }
pContext->mti = MT_N;
return pContext;
}
uint32_t FX_Random_MT_Generate(void* pContext) {
MTContext* pMTC = static_cast<MTContext*>(pContext);
- uint32_t* pBuf = pMTC->mt;
uint32_t v;
if (pMTC->mti >= MT_N) {
- static const uint32_t mag[2] = {0, MT_Matrix_A};
+ static constexpr std::array<uint32_t, 2> mag = {{0, MT_Matrix_A}};
uint32_t kk;
for (kk = 0; kk < MT_N - MT_M; kk++) {
- v = (pBuf[kk] & MT_Upper_Mask) | (pBuf[kk + 1] & MT_Lower_Mask);
- pBuf[kk] = pBuf[kk + MT_M] ^ (v >> 1) ^ mag[v & 1];
+ v = (pMTC->mt[kk] & MT_Upper_Mask) | (pMTC->mt[kk + 1] & MT_Lower_Mask);
+ pMTC->mt[kk] = pMTC->mt[kk + MT_M] ^ (v >> 1) ^ mag[v & 1];
}
for (; kk < MT_N - 1; kk++) {
- v = (pBuf[kk] & MT_Upper_Mask) | (pBuf[kk + 1] & MT_Lower_Mask);
- pBuf[kk] = pBuf[kk + (MT_M - MT_N)] ^ (v >> 1) ^ mag[v & 1];
+ v = (pMTC->mt[kk] & MT_Upper_Mask) | (pMTC->mt[kk + 1] & MT_Lower_Mask);
+ pMTC->mt[kk] = pMTC->mt[kk + (MT_M - MT_N)] ^ (v >> 1) ^ mag[v & 1];
}
- v = (pBuf[MT_N - 1] & MT_Upper_Mask) | (pBuf[0] & MT_Lower_Mask);
- pBuf[MT_N - 1] = pBuf[MT_M - 1] ^ (v >> 1) ^ mag[v & 1];
+ v = (pMTC->mt[MT_N - 1] & MT_Upper_Mask) | (pMTC->mt[0] & MT_Lower_Mask);
+ pMTC->mt[MT_N - 1] = pMTC->mt[MT_M - 1] ^ (v >> 1) ^ mag[v & 1];
pMTC->mti = 0;
}
- v = pBuf[pMTC->mti++];
+ v = pMTC->mt[pMTC->mti++];
v ^= (v >> 11);
v ^= (v << 7) & 0x9d2c5680UL;
v ^= (v << 15) & 0xefc60000UL;
@@ -124,10 +125,10 @@
FX_Free(pContext);
}
-void FX_Random_GenerateMT(uint32_t* pBuffer, int32_t iCount) {
+void FX_Random_GenerateMT(pdfium::span<uint32_t> pBuffer) {
void* pContext = ContextFromNextGlobalSeed();
- while (iCount-- > 0)
- *pBuffer++ = FX_Random_MT_Generate(pContext);
-
+ for (size_t i = 0; i < pBuffer.size(); ++i) {
+ pBuffer[i] = FX_Random_MT_Generate(pContext);
+ }
FX_Random_MT_Close(pContext);
}
diff --git a/core/fxcrt/fx_random.h b/core/fxcrt/fx_random.h
index c6cbd5e..635e8cc 100644
--- a/core/fxcrt/fx_random.h
+++ b/core/fxcrt/fx_random.h
@@ -9,10 +9,12 @@
#include <stdint.h>
+#include "third_party/base/containers/span.h"
+
void* FX_Random_MT_Start(uint32_t dwSeed);
void FX_Random_MT_Close(void* pContext);
uint32_t FX_Random_MT_Generate(void* pContext);
-void FX_Random_GenerateMT(uint32_t* pBuffer, int32_t iCount);
+void FX_Random_GenerateMT(pdfium::span<uint32_t> pBuffer);
#endif // CORE_FXCRT_FX_RANDOM_H_
diff --git a/core/fxcrt/fx_random_unittest.cpp b/core/fxcrt/fx_random_unittest.cpp
index 1361e24..0c6b9b0 100644
--- a/core/fxcrt/fx_random_unittest.cpp
+++ b/core/fxcrt/fx_random_unittest.cpp
@@ -18,7 +18,7 @@
std::set<std::array<uint32_t, 16>> seen;
std::array<uint32_t, 16> current;
for (int i = 0; i < 3600; ++i) {
- FX_Random_GenerateMT(current.data(), 16);
+ FX_Random_GenerateMT(current);
EXPECT_TRUE(seen.insert(current).second);
}
}
diff --git a/fxjs/xfa/cfxjse_formcalc_context.cpp b/fxjs/xfa/cfxjse_formcalc_context.cpp
index b591122..6dc2975 100644
--- a/fxjs/xfa/cfxjse_formcalc_context.cpp
+++ b/fxjs/xfa/cfxjse_formcalc_context.cpp
@@ -24,6 +24,7 @@
#include "core/fxcrt/fx_extension.h"
#include "core/fxcrt/fx_random.h"
#include "core/fxcrt/fx_safe_types.h"
+#include "core/fxcrt/span_util.h"
#include "core/fxcrt/widetext_buffer.h"
#include "fxjs/fxv8.h"
#include "fxjs/xfa/cfxjse_class.h"
@@ -492,7 +493,8 @@
ByteString GUIDString(bool bSeparator) {
uint8_t data[16];
- FX_Random_GenerateMT(reinterpret_cast<uint32_t*>(data), 4);
+ auto random_span = pdfium::make_span(data);
+ FX_Random_GenerateMT(fxcrt::reinterpret_span<uint32_t>(random_span));
data[6] = (data[6] & 0x0F) | 0x40;
ByteString bsGUID;
diff --git a/fxjs/xfa/cfxjse_formcalc_context_embeddertest.cpp b/fxjs/xfa/cfxjse_formcalc_context_embeddertest.cpp
index 602a744..89f651f 100644
--- a/fxjs/xfa/cfxjse_formcalc_context_embeddertest.cpp
+++ b/fxjs/xfa/cfxjse_formcalc_context_embeddertest.cpp
@@ -4,6 +4,9 @@
#include <math.h>
+#include <algorithm>
+
+#include "core/fxcrt/fx_extension.h"
#include "fxjs/fxv8.h"
#include "fxjs/xfa/cfxjse_engine.h"
#include "fxjs/xfa/cfxjse_isolatetracker.h"
@@ -913,7 +916,12 @@
CFXJSE_ScopeUtil_IsolateHandleContext scope(GetJseContext());
v8::Local<v8::Value> value = GetValue();
- EXPECT_TRUE(fxv8::IsString(value));
+ ASSERT_TRUE(fxv8::IsString(value));
+ ByteString bstr = fxv8::ToByteString(isolate(), value.As<v8::String>());
+ EXPECT_EQ(bstr.GetLength(), 32u);
+ EXPECT_TRUE(std::all_of(bstr.begin(), bstr.end(), FXSYS_IsHexDigit));
+ EXPECT_TRUE(
+ std::any_of(bstr.begin(), bstr.end(), [](char c) { return c != '0'; }));
}
TEST_F(CFXJSE_FormCalcContextEmbedderTest, Upper) {
