Better size calculations for struct PixelWeight
This CL implements many of the suggestions I made on
https://pdfium-review.googlesource.com/c/pdfium/+/81650
so that those changes can be made independently.
-- Use safe arithmetic when calculating allocation sizes.
-- Make calculations robust in face of changes to PixelWeight.
-- Add sanity check on `dest_max` argument.
Change-Id: Idd4fef6f6784fbd6175eb7d7927398e0907e9dc5
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/81671
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/core/fxcodec/progressive_decoder.cpp b/core/fxcodec/progressive_decoder.cpp
index f797a4c..764e5fa 100644
--- a/core/fxcodec/progressive_decoder.cpp
+++ b/core/fxcodec/progressive_decoder.cpp
@@ -71,9 +71,12 @@
void ProgressiveDecoder::WeightTable::Calc(int dest_len, int src_len) {
CHECK_GE(dest_len, 0);
double scale = static_cast<double>(src_len) / dest_len;
- m_ItemSize = (int)(sizeof(int) * 2 + sizeof(int) * (ceil(fabs(scale)) + 1));
+ const size_t weight_count = static_cast<size_t>(ceil(fabs(scale))) + 1;
+ m_ItemSize = PixelWeight::TotalBytesForWeightCount(weight_count);
+ FX_SAFE_SIZE_T safe_size = m_ItemSize;
+ safe_size *= dest_len;
+ m_pWeightTables.resize(safe_size.ValueOrDie());
m_DestMin = 0;
- m_pWeightTables.resize(dest_len * m_ItemSize + 4);
if (fabs(scale) < 1.0) {
for (int dest_pixel = 0; dest_pixel < dest_len; dest_pixel++) {
PixelWeight& pixel_weights = *GetPixelWeight(dest_pixel);
@@ -144,10 +147,11 @@
void ProgressiveDecoder::HorzTable::Calc(int dest_len, int src_len) {
CHECK_GE(dest_len, 0);
+ m_ItemSize = PixelWeight::TotalBytesForWeightCount(2);
+ FX_SAFE_SIZE_T safe_size = m_ItemSize;
+ safe_size *= dest_len;
+ m_pWeightTables.resize(safe_size.ValueOrDie(), 0);
double scale = (double)dest_len / (double)src_len;
- m_ItemSize = sizeof(int) * 4;
- int size = dest_len * m_ItemSize + 4;
- m_pWeightTables.resize(size, 0);
if (scale > 1) {
int pre_dest_col = 0;
for (int src_col = 0; src_col < src_len; src_col++) {
@@ -198,10 +202,11 @@
void ProgressiveDecoder::VertTable::Calc(int dest_len, int src_len) {
CHECK_GE(dest_len, 0);
+ m_ItemSize = PixelWeight::TotalBytesForWeightCount(2);
+ FX_SAFE_SIZE_T safe_size = m_ItemSize;
+ safe_size *= dest_len;
+ m_pWeightTables.resize(safe_size.ValueOrDie(), 0);
double scale = (double)dest_len / (double)src_len;
- m_ItemSize = sizeof(int) * 4;
- int size = dest_len * m_ItemSize + 4;
- m_pWeightTables.resize(size, 0);
if (scale <= 1) {
for (int dest_row = 0; dest_row < dest_len; dest_row++) {
PixelWeight* pWeight = GetPixelWeight(dest_row);
diff --git a/core/fxge/dib/cstretchengine.cpp b/core/fxge/dib/cstretchengine.cpp
index fd0aed2..222dd29 100644
--- a/core/fxge/dib/cstretchengine.cpp
+++ b/core/fxge/dib/cstretchengine.cpp
@@ -29,8 +29,8 @@
CStretchEngine::CWeightTable::~CWeightTable() = default;
-size_t CStretchEngine::CWeightTable::GetPixelWeightSize() const {
- return m_ItemSize / sizeof(int) - 2;
+size_t CStretchEngine::CWeightTable::GetPixelWeightCount() const {
+ return PixelWeight::WeightCountFromTotalBytes(m_ItemSizeBytes);
}
bool CStretchEngine::CWeightTable::Calc(int dest_len,
@@ -40,23 +40,35 @@
int src_min,
int src_max,
const FXDIB_ResampleOptions& options) {
+ static constexpr size_t kBytesPerWeight = sizeof(PixelWeight::m_Weights[0]);
+ static constexpr size_t kMaxTableBytesAllowed = (1 << 30) - kBytesPerWeight;
+
// Help the compiler realize that these can't change during a loop iteration:
const bool bilinear = options.bInterpolateBilinear;
+ m_DestMin = 0;
+ m_ItemSizeBytes = 0;
+ m_WeightTablesSizeBytes = 0;
m_WeightTables.clear();
- m_dwWeightTablesSize = 0;
- const double scale = static_cast<float>(src_len) / dest_len;
- const double base = dest_len < 0 ? src_len : 0;
- m_ItemSize = sizeof(int) * 2 +
- static_cast<int>(sizeof(int) *
- (ceil(fabs(static_cast<float>(scale))) + 1));
-
- m_DestMin = dest_min;
- if (dest_max - dest_min > static_cast<int>((1U << 30) - 4) / m_ItemSize)
+ if (dest_min > dest_max)
return false;
- m_dwWeightTablesSize = (dest_max - dest_min) * m_ItemSize + 4;
- m_WeightTables.resize(m_dwWeightTablesSize);
+ m_DestMin = dest_min;
+
+ // TODO(tsepez): test results are sensitive to `scale` being a double
+ // rather than a float with an initial value no more precise than float.
+ const double scale = static_cast<float>(src_len) / dest_len;
+ const double base = dest_len < 0 ? src_len : 0;
+ const size_t weight_count = static_cast<size_t>(ceil(fabs(scale))) + 1;
+ m_ItemSizeBytes = PixelWeight::TotalBytesForWeightCount(weight_count);
+
+ const size_t dest_range = static_cast<size_t>(dest_max - dest_min);
+ const size_t kMaxTableItemsAllowed = kMaxTableBytesAllowed / m_ItemSizeBytes;
+ if (dest_range > kMaxTableItemsAllowed)
+ return false;
+
+ m_WeightTablesSizeBytes = dest_range * m_ItemSizeBytes;
+ m_WeightTables.resize(m_WeightTablesSizeBytes);
if (options.bNoSmoothing || fabs(static_cast<float>(scale)) < 1.0f) {
for (int dest_pixel = dest_min; dest_pixel < dest_max; ++dest_pixel) {
PixelWeight& pixel_weights = *GetPixelWeight(dest_pixel);
@@ -116,7 +128,7 @@
break;
}
size_t idx = j - start_i;
- if (idx >= GetPixelWeightSize())
+ if (idx >= GetPixelWeightCount())
return false;
pixel_weights.m_Weights[idx] = FXSYS_roundf(weight * 65536);
@@ -129,7 +141,7 @@
int pixel) const {
DCHECK(pixel >= m_DestMin);
return reinterpret_cast<const PixelWeight*>(
- &m_WeightTables[(pixel - m_DestMin) * m_ItemSize]);
+ &m_WeightTables[(pixel - m_DestMin) * m_ItemSizeBytes]);
}
int* CStretchEngine::CWeightTable::GetValueFromPixelWeight(PixelWeight* pWeight,
@@ -138,7 +150,7 @@
return nullptr;
size_t idx = index - pWeight->m_SrcStart;
- return idx < GetPixelWeightSize() ? &pWeight->m_Weights[idx] : nullptr;
+ return idx < GetPixelWeightCount() ? &pWeight->m_Weights[idx] : nullptr;
}
CStretchEngine::CStretchEngine(ScanlineComposerIface* pDestBitmap,
diff --git a/core/fxge/dib/cstretchengine.h b/core/fxge/dib/cstretchengine.h
index d631a59..fc23e85 100644
--- a/core/fxge/dib/cstretchengine.h
+++ b/core/fxge/dib/cstretchengine.h
@@ -57,12 +57,12 @@
}
int* GetValueFromPixelWeight(PixelWeight* pWeight, int index) const;
- size_t GetPixelWeightSize() const;
+ size_t GetPixelWeightCount() const;
private:
int m_DestMin = 0;
- int m_ItemSize = 0;
- size_t m_dwWeightTablesSize = 0;
+ size_t m_ItemSizeBytes = 0;
+ size_t m_WeightTablesSizeBytes = 0;
std::vector<uint8_t, FxAllocAllocator<uint8_t>> m_WeightTables;
};
diff --git a/core/fxge/dib/fx_dib.cpp b/core/fxge/dib/fx_dib.cpp
index 1f67cd8..2d50e83 100644
--- a/core/fxge/dib/fx_dib.cpp
+++ b/core/fxge/dib/fx_dib.cpp
@@ -11,6 +11,7 @@
#include "build/build_config.h"
#include "core/fxcrt/fx_extension.h"
+#include "core/fxcrt/fx_safe_types.h"
#if defined(OS_WIN)
static_assert(sizeof(FX_COLORREF) == sizeof(COLORREF),
@@ -32,6 +33,22 @@
}
}
+// static
+size_t PixelWeight::TotalBytesForWeightCount(size_t weight_count) {
+ const size_t extra_weights = weight_count > 0 ? weight_count - 1 : 0;
+ FX_SAFE_SIZE_T total_bytes = extra_weights;
+ total_bytes *= sizeof(m_Weights[0]);
+ total_bytes += sizeof(PixelWeight);
+ return total_bytes.ValueOrDie();
+}
+
+// static
+size_t PixelWeight::WeightCountFromTotalBytes(size_t total_bytes) {
+ const size_t extra_bytes =
+ total_bytes > sizeof(PixelWeight) ? total_bytes - sizeof(PixelWeight) : 0;
+ return 1 + extra_bytes / sizeof(m_Weights[0]);
+}
+
FXDIB_ResampleOptions::FXDIB_ResampleOptions() = default;
bool FXDIB_ResampleOptions::HasAnyOptions() const {
diff --git a/core/fxge/dib/fx_dib.h b/core/fxge/dib/fx_dib.h
index 6ea7502..d88bd79 100644
--- a/core/fxge/dib/fx_dib.h
+++ b/core/fxge/dib/fx_dib.h
@@ -30,9 +30,12 @@
};
struct PixelWeight {
+ static size_t TotalBytesForWeightCount(size_t weight_count);
+ static size_t WeightCountFromTotalBytes(size_t total_bytes);
+
int m_SrcStart;
int m_SrcEnd;
- int m_Weights[1];
+ int m_Weights[1]; // Not really 1, variable size.
};
using FX_ARGB = uint32_t;
