Commit 103dc134 for tesseract
commit 103dc134eb36411ddc6833ec20aa2c76795bd0ff
Author: Stefan Weil <sw@weilnetz.de>
Date: Fri Aug 21 15:23:59 2026 +0200
Validate FullyConnected weight matrix dimensions at load
FullyConnected::DeSerialize read the WeightMatrix without checking
that its dimensions match the layer's declared ni/no. MatrixDotVector
drives the dot product from the matrix dimensions (writes w.dim1()
results, reads w.dim2()-1 inputs) while the scratch buffers are sized
from no_ and ni_, so a crafted .traineddata with a mismatched matrix
performed a heap out-of-bounds write (up to 65535 rows) and out-of-
bounds read on the first recognition step (crash, or heap corruption
with attacker-influenced size and content).
Key changes:
- weightmatrix.h: add Dim1()/Dim2() accessors for the active weight
matrix (int or float), alongside the existing NumOutputs().
- fullyconnected.cpp: reject the layer in DeSerialize unless
Dim1() == no_ and Dim2() == ni_ + 1 (the second dimension
includes the bias column), the layout that InitWeightsFloat
always produces.
- unittest: new fullyconnected_test that builds a Softmax network
with mismatched matrix dimensions and expects CreateFromFile to
return nullptr; on unpatched code the test reaches Forward and
ASan catches the out-of-bounds write in MatrixDotVector. A
second test verifies that matching dimensions are still accepted.
Reported-by: Zhixi "Jace" Sun <g.mygenie@gmail.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 08925be0..28beec09 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1158,6 +1158,7 @@ if !DISABLED_LEGACY_ENGINE
check_PROGRAMS += equationdetect_test
endif # !DISABLED_LEGACY_ENGINE
check_PROGRAMS += fileio_test
+check_PROGRAMS += fullyconnected_test
check_PROGRAMS += heap_test
check_PROGRAMS += imagedata_test
if !DISABLED_LEGACY_ENGINE
@@ -1291,6 +1292,10 @@ fileio_test_SOURCES = unittest/fileio_test.cc
fileio_test_CPPFLAGS = $(unittest_CPPFLAGS)
fileio_test_LDADD = $(TRAINING_LIBS)
+fullyconnected_test_SOURCES = unittest/fullyconnected_test.cc
+fullyconnected_test_CPPFLAGS = $(unittest_CPPFLAGS)
+fullyconnected_test_LDADD = $(TESS_LIBS)
+
heap_test_SOURCES = unittest/heap_test.cc
heap_test_CPPFLAGS = $(unittest_CPPFLAGS)
heap_test_LDADD = $(TESS_LIBS)
diff --git a/src/lstm/fullyconnected.cpp b/src/lstm/fullyconnected.cpp
index a0ee1b02..d999f575 100644
--- a/src/lstm/fullyconnected.cpp
+++ b/src/lstm/fullyconnected.cpp
@@ -121,7 +121,16 @@ bool FullyConnected::Serialize(TFile *fp) const {
// Reads from the given file. Returns false in case of error.
bool FullyConnected::DeSerialize(TFile *fp) {
- return weights_.DeSerialize(IsTraining(), fp);
+ if (!weights_.DeSerialize(IsTraining(), fp)) {
+ return false;
+ }
+ // The weight matrix must match the declared sizes (the second dimension
+ // includes the bias column); otherwise Forward would read or write
+ // outside the scratch buffers sized from ni_ and no_.
+ if (weights_.Dim1() != no_ || weights_.Dim2() != ni_ + 1) {
+ return false;
+ }
+ return true;
}
// Runs forward propagation of activations on the input line.
diff --git a/src/lstm/weightmatrix.h b/src/lstm/weightmatrix.h
index eaca3ffb..6c5ab236 100644
--- a/src/lstm/weightmatrix.h
+++ b/src/lstm/weightmatrix.h
@@ -107,6 +107,13 @@ public:
int NumOutputs() const {
return int_mode_ ? wi_.dim1() : wf_.dim1();
}
+ // The dimensions of the active weight matrix (wi_ in int mode, else wf_).
+ int Dim1() const {
+ return int_mode_ ? wi_.dim1() : wf_.dim1();
+ }
+ int Dim2() const {
+ return int_mode_ ? wi_.dim2() : wf_.dim2();
+ }
// Provides one set of weights. Only used by peep weight maxpool.
const TFloat *GetWeights(int index) const {
return wf_[index];
diff --git a/unittest/fullyconnected_test.cc b/unittest/fullyconnected_test.cc
new file mode 100644
index 00000000..f9697de4
--- /dev/null
+++ b/unittest/fullyconnected_test.cc
@@ -0,0 +1,115 @@
+///////////////////////////////////////////////////////////////////////
+// File: fullyconnected_test.cc
+// Description: Tests that a FullyConnected (softmax) network layer with
+// weight-matrix dimensions that do not match the declared
+// ni/no is rejected at load. Without the check,
+// MatrixDotVector writes w.dim1() results into a scratch
+// buffer sized from no_ and reads w.dim2()-1 inputs from
+// a buffer sized from ni_ (heap out-of-bounds write/read)
+// on the first recognition step.
+//
+// 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 "network.h" // for Network, NetworkType
+#include "networkio.h"
+#include "networkscratch.h"
+#include "serialis.h" // for TFile
+
+#include <cstdint>
+#include <vector>
+
+namespace tesseract {
+namespace {
+
+// Appends raw little-endian values to a byte buffer.
+class ByteWriter {
+public:
+ void PutU8(uint32_t v) { data_.push_back(static_cast<char>(v & 0xFF)); }
+ void PutU32(uint32_t v) {
+ for (int i = 0; i < 4; ++i) {
+ data_.push_back(static_cast<char>((v >> (8 * i)) & 0xFF));
+ }
+ }
+ void PutS32(int32_t v) { PutU32(static_cast<uint32_t>(v)); }
+ const std::vector<char> &data() const { return data_; }
+
+private:
+ std::vector<char> data_;
+};
+
+void PutDoubleLE(ByteWriter *w, double d) {
+ union {
+ double d;
+ uint64_t u;
+ } conv;
+ conv.d = d;
+ w->PutU32(static_cast<uint32_t>(conv.u & 0xFFFFFFFF));
+ w->PutU32(static_cast<uint32_t>(conv.u >> 32));
+}
+
+// Builds a serialized NT_SOFTMAX network: header with the given ni/no,
+// then a float-mode WeightMatrix with the given (corrupt) dimensions.
+// The matrix is stored as doubles on disk (see WeightMatrix::DeSerialize).
+std::vector<char> MakeSoftmaxNetwork(int ni, int no, int32_t dim1, int32_t dim2) {
+ ByteWriter w;
+ w.PutU8(static_cast<uint32_t>(NT_SOFTMAX));
+ w.PutU8(0); // training: TS_DISABLED
+ w.PutU8(0); // needs_to_backprop
+ w.PutU32(0); // network_flags
+ w.PutU32(static_cast<uint32_t>(ni));
+ w.PutU32(static_cast<uint32_t>(no));
+ w.PutU32(0); // num_weights (not cross-checked, kept consistent anyway)
+ w.PutU32(0); // name (empty string)
+ // WeightMatrix::DeSerialize:
+ w.PutU8(128); // mode: kDoubleFlag, float mode
+ w.PutS32(dim1);
+ w.PutS32(dim2);
+ PutDoubleLE(&w, 0.0); // empty_ cell
+ for (int32_t i = 0; i < dim1 * dim2; ++i) {
+ PutDoubleLE(&w, 0.0); // weight data
+ }
+ return w.data();
+}
+
+// A FullyConnected layer whose weight matrix does not match the declared
+// sizes must be rejected by CreateFromFile; on unpatched code the test
+// reaches Forward, where MatrixDotVector performs the out-of-bounds
+// write this regression test guards against.
+TEST(FullyconnectedTest, RejectsWeightMatrixDimensionMismatch) {
+ // ni_=no_=1 but dim1=3 (OOB write of 3 results into a 1-result buffer)
+ // and dim2=5 (OOB read of 4 inputs from a 1-input buffer).
+ std::vector<char> bytes = MakeSoftmaxNetwork(1, 1, 3, 5);
+ TFile fp;
+ ASSERT_TRUE(fp.Open(bytes.data(), bytes.size()));
+ Network *net = Network::CreateFromFile(&fp);
+ if (net == nullptr) {
+ return; // Fixed: the mismatched layer is rejected at load.
+ }
+ NetworkIO input;
+ input.Resize2d(false, /*width=*/1, /*num_features=*/1);
+ NetworkScratch scratch;
+ NetworkIO output;
+ net->Forward(false, input, nullptr, &scratch, &output);
+ delete net;
+ FAIL() << "crafted FullyConnected layer with mismatched weight matrix was accepted";
+}
+
+// Consistent dimensions must still be accepted.
+TEST(FullyconnectedTest, AcceptsMatchingDimensions) {
+ std::vector<char> bytes = MakeSoftmaxNetwork(1, 2, 2, 2);
+ TFile fp;
+ ASSERT_TRUE(fp.Open(bytes.data(), bytes.size()));
+ Network *net = Network::CreateFromFile(&fp);
+ ASSERT_NE(net, nullptr);
+ delete net;
+}
+
+} // namespace
+} // namespace tesseract