Commit 048e9237 for tesseract
commit 048e92378f1a34dcdaf94bc515c7552ac01f08ef
Author: Stefan Weil <sw@weilnetz.de>
Date: Sun Oct 18 20:11:03 2026 +0200
Reject malformed dimensions in GENERIC_2D_ARRAY deserialization (#4649)
The TFile-based DeSerializeSize read the matrix dimensions as int32_t
and validated them with a signed comparison against UINT16_MAX, so a
negative dimension (e.g. 0xFFFFFFFF = -1) passed the check and
reached Resize, where it caused signed integer overflow in
ResizeNoInit (size1 * size2) and out-of-bounds accesses in
IntSimdMatrix when loading a crafted .traineddata file.
Read the dimensions as uint32_t, like the FILE * variant above
already does: the same bytes then fail the UINT16_MAX bound check
instead of passing as a negative value. Also require that the
declared element count is backed by the bytes that actually remain
in the stream, so a crafted file cannot declare gigabytes of data
while carrying a few hundred bytes and force a huge allocation
(denial of service) from a tiny input.
Bound ResizeNoInit against the same denial-of-service vector for
all callers: compute the element count in 64 bits so it cannot
overflow (in particular (-1) * (-1) must not pass as 1), reject
negative dimensions, and cap the total element count at 100000000,
consistent with the "unreasonably large" bound already applied to
the scale vector in WeightMatrix::DeSerialize.
This follows up on GHSA-9vwv-49m8-jv9m, whose localized dim2 < 1
check in WeightMatrix::DeSerialize did not cover the other callers
of DeSerializeSize or the allocation-size DoS.
Adds matrix_deserializesize_test covering the valid, negative,
overflowing-product, oversized and unbacked-declared-size cases
for deserialization, plus the ResizeNoInit cap, negative and
overflow cases.
Reported-by: Tristan Madani <tristan@talencesecurity.com>
Assisted-by: OpenCode / qwen3.8-27b-thinking (Alibaba Cloud)
Signed-off-by: Stefan Weil <sw@weilnetz.de>
diff --git a/Makefile.am b/Makefile.am
index e756048c..e268b50d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1187,6 +1187,7 @@ check_PROGRAMS += loadlang_test
if !DISABLED_LEGACY_ENGINE
check_PROGRAMS += mastertrainer_test
endif # !DISABLED_LEGACY_ENGINE
+check_PROGRAMS += matrix_deserializesize_test
check_PROGRAMS += matrix_test
check_PROGRAMS += networkio_test
if !DISABLED_LEGACY_ENGINE
@@ -1407,6 +1408,10 @@ mastertrainer_test_CPPFLAGS = $(unittest_CPPFLAGS)
mastertrainer_test_LDADD = $(TRAINING_LIBS) $(LEPTONICA_LIBS)
endif # !DISABLED_LEGACY_ENGINE
+matrix_deserializesize_test_SOURCES = unittest/matrix_deserializesize_test.cc
+matrix_deserializesize_test_CPPFLAGS = $(unittest_CPPFLAGS)
+matrix_deserializesize_test_LDADD = $(TESS_LIBS)
+
matrix_test_SOURCES = unittest/matrix_test.cc
matrix_test_CPPFLAGS = $(unittest_CPPFLAGS)
matrix_test_LDADD = $(TESS_LIBS)
diff --git a/src/ccstruct/matrix.h b/src/ccstruct/matrix.h
index dba89eda..f53f17ce 100644
--- a/src/ccstruct/matrix.h
+++ b/src/ccstruct/matrix.h
@@ -111,12 +111,28 @@ public:
}
}
+ // DoS protection: the maximum number of elements (including the
+ // padding) a single array may hold. Legitimate matrices in tesseract
+ // are far smaller than this; the cap is consistent with the
+ // "unreasonably large" bound applied to the scale vector in
+ // WeightMatrix::DeSerialize.
+ static const int64_t kMaxElements = 100000000;
+
// Reallocates the array to the given size. Does not keep old data, but does
// not initialize the array either.
// The allocated memory is expanded on the end by pad, allowing deliberate
// access beyond the bounds of the array.
void ResizeNoInit(int size1, int size2, int pad = 0) {
- int new_size = size1 * size2 + pad;
+ // Compute the total element count in 64 bits so it cannot overflow
+ // (in particular (-1) * (-1) must not pass as 1), then reject
+ // corrupted values (a crafted .traineddata file could produce
+ // negative or oversized dimensions) and bound the allocation against
+ // denial of service.
+ const int64_t total = static_cast<int64_t>(size1) * size2 + pad;
+ if (size1 < 0 || size2 < 0 || pad < 0 || total > kMaxElements) {
+ return;
+ }
+ const int new_size = static_cast<int>(total);
if (new_size > size_allocated_) {
delete[] array_;
array_ = new T[new_size];
@@ -125,7 +141,7 @@ public:
dim1_ = size1;
dim2_ = size2;
// Fill the padding data so it isn't uninitialized.
- for (int i = size1 * size2; i < new_size; ++i) {
+ for (int i = new_size - pad; i < new_size; ++i) {
array_[i] = empty_;
}
}
@@ -589,7 +605,14 @@ protected:
return true;
}
bool DeSerializeSize(TFile *fp) {
- int32_t size1, size2;
+ // Read the dimensions as uint32_t, like the FILE * variant above.
+ // The previous int32_t read compared the signed value against the
+ // signed upper bound, so a negative dimension (e.g. 0xFFFFFFFF = -1)
+ // passed the check, later causing signed integer overflow in
+ // ResizeNoInit (size1 * size2) and out-of-bounds accesses in
+ // IntSimdMatrix. Read as unsigned, the same bytes are 0xFFFFFFFF
+ // and fail the bound check below.
+ uint32_t size1, size2;
if (!fp->DeSerialize(&size1)) {
return false;
}
@@ -603,6 +626,16 @@ protected:
if (size2 > UINT16_MAX) {
return false;
}
+ // The caller allocates before reading the elements, so the declared
+ // size must be backed by the bytes that actually remain in the stream
+ // (empty_ plus the element data). Without this check a crafted file
+ // could declare gigabytes of data while carrying a few hundred bytes,
+ // forcing a huge allocation (denial of service) from a tiny input.
+ const uint64_t required = (static_cast<uint64_t>(size1) * size2 + 1) *
+ static_cast<uint64_t>(sizeof(T));
+ if (required > fp->RemainingBytes()) {
+ return false;
+ }
Resize(size1, size2, empty_);
return true;
}
diff --git a/unittest/matrix_deserializesize_test.cc b/unittest/matrix_deserializesize_test.cc
new file mode 100644
index 00000000..8dbab22b
--- /dev/null
+++ b/unittest/matrix_deserializesize_test.cc
@@ -0,0 +1,163 @@
+///////////////////////////////////////////////////////////////////////
+// File: matrix_deserializesize_test.cc
+// Description: Tests that GENERIC_2D_ARRAY::DeSerializeSize rejects
+// negative and oversized dimensions read from a TFile
+// stream. The dimensions are serialized as little-endian
+// int32, so a crafted .traineddata can supply values such
+// as -1 (0xFFFFFFFF) or 65535 that the previous signed
+// "size > UINT16_MAX" check failed to reject. The -1 case
+// made ResizeNoInit compute (-1)*(-1) = 1 and leave the
+// array indexed as dim1_ = dim2_ = -1, while 65535*65535
+// overflowed the 32-bit int used for the element count.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+///////////////////////////////////////////////////////////////////////
+
+#include "include_gunit.h"
+
+#include "matrix.h" // for GENERIC_2D_ARRAY
+#include "serialis.h" // for TFile
+
+#include <cstdint>
+#include <vector>
+
+namespace tesseract {
+namespace {
+
+// Appends little-endian values to a byte buffer, mirroring the layout
+// GENERIC_2D_ARRAY<T>::DeSerialize(TFile*) expects:
+// int32 dim1, int32 dim2, T empty_, dim1*dim2 T cells.
+class MxWriter {
+ public:
+ void PutI32(int32_t v) {
+ for (int i = 0; i < 4; ++i) data_.push_back(static_cast<char>((v >> (8 * i)) & 0xFF));
+ }
+ void PutU8(uint8_t v) { data_.push_back(static_cast<char>(v)); }
+ const std::vector<char> &data() const { return data_; }
+
+ private:
+ std::vector<char> data_;
+};
+
+// Builds a GENERIC_2D_ARRAY<int8_t> blob with the given (possibly
+// negative or oversized) dimensions and num_cells payload bytes.
+std::vector<char> MakeMatrixBlob(int32_t dim1, int32_t dim2, int32_t num_cells) {
+ MxWriter w;
+ w.PutI32(dim1);
+ w.PutI32(dim2);
+ w.PutU8(0); // empty_
+ for (int32_t i = 0; i < num_cells; ++i) {
+ w.PutU8(0);
+ }
+ return w.data();
+}
+
+bool DeSerializeMatrix(const std::vector<char> &blob, GENERIC_2D_ARRAY<int8_t> *m) {
+ TFile fp;
+ fp.Open(blob.data(), blob.size());
+ if (fp.RemainingBytes() != blob.size()) {
+ return false;
+ }
+ return m->DeSerialize(&fp);
+}
+
+class MatrixDeserializeSizeTest : public testing::Test {
+};
+
+// A valid small matrix must still deserialize successfully.
+TEST_F(MatrixDeserializeSizeTest, AcceptsValidMatrix) {
+ GENERIC_2D_ARRAY<int8_t> m;
+ ASSERT_TRUE(DeSerializeMatrix(MakeMatrixBlob(2, 3, 6), &m));
+ EXPECT_EQ(2, m.dim1());
+ EXPECT_EQ(3, m.dim2());
+}
+
+// A negative dimension must be rejected instead of being passed to
+// Resize, where (-1)*(-1) would allocate a 1-element array while the
+// dims stay negative (later used as OOB indices).
+TEST_F(MatrixDeserializeSizeTest, RejectsNegativeDim) {
+ GENERIC_2D_ARRAY<int8_t> m;
+ EXPECT_FALSE(DeSerializeMatrix(MakeMatrixBlob(-1, -1, 1), &m));
+}
+
+// A negative dimension in either position must be rejected.
+TEST_F(MatrixDeserializeSizeTest, RejectsNegativeDim2) {
+ GENERIC_2D_ARRAY<int8_t> m;
+ EXPECT_FALSE(DeSerializeMatrix(MakeMatrixBlob(3, -1, 0), &m));
+}
+
+// Both dimensions at UINT16_MAX must be rejected: the product
+// 65535*65535 does not fit in a 32-bit signed int, so it would wrap to
+// a negative element count.
+TEST_F(MatrixDeserializeSizeTest, RejectsOverflowingProduct) {
+ GENERIC_2D_ARRAY<int8_t> m;
+ EXPECT_FALSE(DeSerializeMatrix(MakeMatrixBlob(65535, 65535, 0), &m));
+}
+
+// A dimension above UINT16_MAX must still be rejected (regression:
+// this was already checked before, keep it).
+TEST_F(MatrixDeserializeSizeTest, RejectsOversizedDim) {
+ GENERIC_2D_ARRAY<int8_t> m;
+ EXPECT_FALSE(DeSerializeMatrix(MakeMatrixBlob(65536, 1, 0), &m));
+}
+
+// A declared element count that fits in a 32-bit int but is not backed
+// by the bytes in the stream must be rejected instead of forcing a
+// huge allocation (denial of service from a tiny crafted file).
+TEST_F(MatrixDeserializeSizeTest, RejectsDeclaredSizeLargerThanStream) {
+ // 32768 * 65535 = 2147450880 < INT32_MAX, but the blob carries only a
+ // handful of payload bytes.
+ GENERIC_2D_ARRAY<int8_t> m;
+ EXPECT_FALSE(DeSerializeMatrix(MakeMatrixBlob(32768, 65535, 8), &m));
+}
+
+// A large matrix whose declared size is fully present in the stream
+// must still deserialize successfully.
+TEST_F(MatrixDeserializeSizeTest, AcceptsLargeFullyBackedMatrix) {
+ GENERIC_2D_ARRAY<int8_t> m;
+ ASSERT_TRUE(DeSerializeMatrix(MakeMatrixBlob(65535, 1, 65535), &m));
+ EXPECT_EQ(65535, m.dim1());
+ EXPECT_EQ(1, m.dim2());
+}
+
+// ResizeNoInit itself must bound the element count, for callers that
+// have no stream to check against. The maximum allowed size must
+// still be accepted.
+TEST_F(MatrixDeserializeSizeTest, ResizeNoInitAcceptsMaxSize) {
+ GENERIC_2D_ARRAY<int8_t> m;
+ m.ResizeNoInit(10000, 10000); // exactly kMaxElements
+ EXPECT_EQ(10000, m.dim1());
+ EXPECT_EQ(10000, m.dim2());
+}
+
+// A size above the DoS cap must be rejected even though the product
+// fits in a 32-bit int (10001 * 10000 = 100010000).
+TEST_F(MatrixDeserializeSizeTest, ResizeNoInitRejectsOverCap) {
+ GENERIC_2D_ARRAY<int8_t> m;
+ m.ResizeNoInit(10001, 10000);
+ EXPECT_EQ(0, m.dim1());
+ EXPECT_EQ(0, m.dim2());
+}
+
+// A negative size must be rejected (no allocation, no state change).
+TEST_F(MatrixDeserializeSizeTest, ResizeNoInitRejectsNegative) {
+ GENERIC_2D_ARRAY<int8_t> m;
+ m.ResizeNoInit(-1, -1);
+ EXPECT_EQ(0, m.dim1());
+ EXPECT_EQ(0, m.dim2());
+}
+
+// The 65535 x 65535 overflow case must be rejected as well.
+TEST_F(MatrixDeserializeSizeTest, ResizeNoInitRejectsOverflowingProduct) {
+ GENERIC_2D_ARRAY<int8_t> m;
+ m.ResizeNoInit(65535, 65535);
+ EXPECT_EQ(0, m.dim1());
+ EXPECT_EQ(0, m.dim2());
+}
+
+} // namespace
+} // namespace tesseract