Fix some unsafe-looking code in CBC_CommonByteMatrix
- introduce bounds checking
- use size_t for sizes.
- rename clear() to Fill() to match STL notions.
Change-Id: I84029962f50d62869e54e39f4a3b205c9ed2f63a
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/91051
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fxbarcode/common/BC_CommonByteMatrix.cpp b/fxbarcode/common/BC_CommonByteMatrix.cpp
index d299d4b..baba324 100644
--- a/fxbarcode/common/BC_CommonByteMatrix.cpp
+++ b/fxbarcode/common/BC_CommonByteMatrix.cpp
@@ -24,28 +24,30 @@
#include <algorithm>
#include <iterator>
#include "core/fxcrt/stl_util.h"
+#include "third_party/base/check_op.h"
-CBC_CommonByteMatrix::CBC_CommonByteMatrix(int32_t width, int32_t height)
+CBC_CommonByteMatrix::CBC_CommonByteMatrix(size_t width, size_t height)
: m_width(width), m_height(height) {
- m_bytes =
- fxcrt::Vector2D<uint8_t, FxAllocAllocator<uint8_t>>(m_height, m_width);
- clear(0xff);
+ static constexpr size_t kMaxBytes = 256 * 1024 * 1024; // 256 MB.
+ CHECK_LT(width, kMaxBytes / height);
+ m_bytes = fxcrt::Vector2D<uint8_t, FxAllocAllocator<uint8_t>>(height, width);
+ Fill(0xff);
}
CBC_CommonByteMatrix::~CBC_CommonByteMatrix() = default;
-uint8_t CBC_CommonByteMatrix::Get(int32_t x, int32_t y) const {
- return m_bytes[y * m_width + x];
+uint8_t CBC_CommonByteMatrix::Get(size_t x, size_t y) const {
+ const size_t offset = y * m_width + x;
+ CHECK_LT(offset, m_bytes.size());
+ return m_bytes[offset];
}
-void CBC_CommonByteMatrix::Set(int32_t x, int32_t y, int32_t value) {
- m_bytes[y * m_width + x] = (uint8_t)value;
+void CBC_CommonByteMatrix::Set(size_t x, size_t y, uint8_t value) {
+ const size_t offset = y * m_width + x;
+ CHECK_LT(offset, m_bytes.size());
+ m_bytes[offset] = value;
}
-void CBC_CommonByteMatrix::Set(int32_t x, int32_t y, uint8_t value) {
- m_bytes[y * m_width + x] = value;
-}
-
-void CBC_CommonByteMatrix::clear(uint8_t value) {
+void CBC_CommonByteMatrix::Fill(uint8_t value) {
std::fill(std::begin(m_bytes), std::end(m_bytes), value);
}
diff --git a/fxbarcode/common/BC_CommonByteMatrix.h b/fxbarcode/common/BC_CommonByteMatrix.h
index 0b95f73..970acf8 100644
--- a/fxbarcode/common/BC_CommonByteMatrix.h
+++ b/fxbarcode/common/BC_CommonByteMatrix.h
@@ -16,21 +16,20 @@
class CBC_CommonByteMatrix final {
public:
- CBC_CommonByteMatrix(int32_t width, int32_t height);
+ CBC_CommonByteMatrix(size_t width, size_t height);
~CBC_CommonByteMatrix();
- int32_t GetWidth() const { return m_width; }
- int32_t GetHeight() const { return m_height; }
+ size_t GetWidth() const { return m_width; }
+ size_t GetHeight() const { return m_height; }
pdfium::span<const uint8_t> GetArray() const { return m_bytes; }
- uint8_t Get(int32_t x, int32_t y) const;
- void Set(int32_t x, int32_t y, int32_t value);
- void Set(int32_t x, int32_t y, uint8_t value);
- void clear(uint8_t value);
+ uint8_t Get(size_t x, size_t y) const;
+ void Set(size_t x, size_t y, uint8_t value);
+ void Fill(uint8_t value);
private:
- int32_t m_width;
- int32_t m_height;
+ const size_t m_width;
+ const size_t m_height;
std::vector<uint8_t, FxAllocAllocator<uint8_t>> m_bytes;
};
diff --git a/fxbarcode/qrcode/BC_QRCoder.cpp b/fxbarcode/qrcode/BC_QRCoder.cpp
index 46f80fc..02ab3e4 100644
--- a/fxbarcode/qrcode/BC_QRCoder.cpp
+++ b/fxbarcode/qrcode/BC_QRCoder.cpp
@@ -26,6 +26,7 @@
#include "fxbarcode/qrcode/BC_QRCoder.h"
#include "fxbarcode/qrcode/BC_QRCoderErrorCorrectionLevel.h"
#include "fxbarcode/qrcode/BC_QRCoderMode.h"
+#include "third_party/base/numerics/safe_conversions.h"
CBC_QRCoder::CBC_QRCoder() = default;
@@ -69,7 +70,8 @@
m_numECBytes != -1 && m_numRSBlocks != -1 &&
IsValidMaskPattern(m_maskPattern) &&
m_numTotalBytes == m_numDataBytes + m_numECBytes && m_matrix &&
- m_matrixWidth == m_matrix->GetWidth() &&
+ m_matrixWidth ==
+ pdfium::base::checked_cast<int32_t>(m_matrix->GetWidth()) &&
m_matrix->GetWidth() == m_matrix->GetHeight();
}
diff --git a/fxbarcode/qrcode/BC_QRCoderMatrixUtil.cpp b/fxbarcode/qrcode/BC_QRCoderMatrixUtil.cpp
index 11008d9..23d76f9 100644
--- a/fxbarcode/qrcode/BC_QRCoderMatrixUtil.cpp
+++ b/fxbarcode/qrcode/BC_QRCoderMatrixUtil.cpp
@@ -106,7 +106,7 @@
if (x == 6)
x -= 1;
- while (y >= 0 && y < matrix->GetHeight()) {
+ while (y >= 0 && y < static_cast<int32_t>(matrix->GetHeight())) {
if (y == 6) {
y += direction;
continue;
@@ -215,8 +215,8 @@
}
bool EmbedTimingPatterns(CBC_CommonByteMatrix* matrix) {
- for (int32_t i = 8; i < matrix->GetWidth() - 8; i++) {
- int32_t bit = (i + 1) % 2;
+ for (size_t i = 8; i + 8 < matrix->GetWidth(); i++) {
+ const uint8_t bit = static_cast<uint8_t>((i + 1) % 2);
if (!IsValidValue(matrix->Get(i, 6)))
return false;
@@ -378,7 +378,7 @@
if (!dataBits || !matrix)
return false;
- matrix->clear(0xff);
+ matrix->Fill(0xff);
if (!EmbedBasicPatterns(version, matrix))
return false;
