From 7b339302be3d41eb9f76478587f336a5be64f3bb Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 01:05:09 +0200 Subject: [PATCH 1/3] fix: bound formula arrays before dimension arithmetic --- CHANGELOG.md | 3 ++ .../internal/formula/formula_evaluator.cpp | 42 +++++++++++-------- .../formula/formula_evaluator_test.cpp | 11 +++++ 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a84a9aad..1422b53f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- Formula array limits are checked before dimension arithmetic can overflow + on 32-bit targets, including broadcast results and explicit array literals. + - Spreadsheet formulas keep quoted sheet spans when rewritten, read bare whole-row ranges, and report cross-sheet ranges as unresolved. Extreme reference shifts give `#REF!` without signed overflow. diff --git a/src/odr/internal/formula/formula_evaluator.cpp b/src/odr/internal/formula/formula_evaluator.cpp index 1021b67fa..66d3b05d8 100644 --- a/src/odr/internal/formula/formula_evaluator.cpp +++ b/src/odr/internal/formula/formula_evaluator.cpp @@ -17,6 +17,15 @@ namespace str = util::string; namespace { +/// Checks array size before narrowing or allocating, including on 32-bit hosts. +std::size_t array_size(const std::uint64_t columns, const std::uint64_t rows) { + constexpr std::uint64_t limit = 1 << 20; + if (columns > limit || rows > limit || columns * rows > limit) { + throw NoAnswer{}; + } + return static_cast(columns * rows); +} + /// The last index of an axis that states @p count positions. std::uint32_t last_of(const std::uint32_t count) { return count == 0 ? 0 : count - 1; @@ -176,9 +185,6 @@ class Evaluator final { /// The most names one inside the other, so a name that names itself ends. static constexpr std::uint32_t name_limit = 16; - /// The most cells an array context reads out of one range. - static constexpr std::size_t array_limit = 1 << 20; - /// @p value as an array: the cells of a range, an array, or one value. [[nodiscard]] Matrix matrix_of(Value value) const { if (auto *matrix = std::get_if(&value.content)) { @@ -196,14 +202,15 @@ class Evaluator final { if (area.whole_columns || area.whole_rows) { throw NoAnswer{}; } - const std::uint32_t columns = - area.range.to().column - area.range.from().column + 1; - const std::uint32_t rows = area.range.to().row - area.range.from().row + 1; - if (std::size_t{columns} * rows > array_limit) { - throw NoAnswer{}; - } - Matrix result{columns, rows, {}}; - result.cells.reserve(std::size_t{columns} * rows); + const std::uint64_t columns = + std::uint64_t{area.range.to().column} - area.range.from().column + 1; + const std::uint64_t rows = + std::uint64_t{area.range.to().row} - area.range.from().row + 1; + const std::size_t count = array_size(columns, rows); + Matrix result{static_cast(columns), + static_cast(rows), + {}}; + result.cells.reserve(count); for (std::uint32_t row = 0; row < rows; ++row) { for (std::uint32_t column = 0; column < columns; ++column) { result.cells.push_back( @@ -285,7 +292,11 @@ class Evaluator final { [[nodiscard]] Value value_of(const ArrayLiteral &array, const Node &node) { Matrix matrix{array.columns, array.rows, {}}; - matrix.cells.reserve(node.children.size()); + const std::size_t count = array_size(array.columns, array.rows); + if (count != node.children.size()) { + throw NoAnswer{}; + } + matrix.cells.reserve(count); for (const Node &child : node.children) { matrix.cells.push_back(scalar(value(child))); } @@ -493,15 +504,12 @@ class Evaluator final { if (c >= matrix->columns || r >= matrix->rows) { return Value{ErrorType::not_available}; } - return matrix->cells[r * matrix->columns + c]; + return matrix->cells[std::size_t{r} * matrix->columns + c]; }; Matrix result{std::max(columns_of(left), columns_of(right)), std::max(rows_of(left), rows_of(right)), {}}; - if (std::size_t{result.columns} * result.rows > array_limit) { - throw NoAnswer{}; - } - result.cells.reserve(std::size_t{result.columns} * result.rows); + result.cells.reserve(array_size(result.columns, result.rows)); for (std::uint32_t row = 0; row < result.rows; ++row) { for (std::uint32_t column = 0; column < result.columns; ++column) { result.cells.push_back(operation(element(a, left, column, row), diff --git a/test/src/internal/formula/formula_evaluator_test.cpp b/test/src/internal/formula/formula_evaluator_test.cpp index 1de29bd78..04563c874 100644 --- a/test/src/internal/formula/formula_evaluator_test.cpp +++ b/test/src/internal/formula/formula_evaluator_test.cpp @@ -8,6 +8,7 @@ #include #include +#include using namespace odr; using namespace odr::internal; @@ -231,3 +232,13 @@ TEST(FormulaEvaluator, an_excel_comparison_near_the_15th_digit_has_no_answer) { EXPECT_EQ(ods("=1+4E-15=1"), boolean(false)); EXPECT_EQ(xlsx("=0.1+0.2=0.3"), boolean(true)); } + +TEST(FormulaEvaluator, array_limits_apply_before_dimension_products_narrow) { + EXPECT_EQ(xlsx("=SUMPRODUCT(A1:CRXP65536)"), std::nullopt); + Node row{ArrayLiteral{65536, 1}, {}}; + row.children.assign(65536, Node{NumberLiteral{1}, {}}); + Node column{ArrayLiteral{1, 65536}, row.children}; + const Node sum{BinaryOperation{BinaryOperator::add}, + {std::move(row), std::move(column)}}; + EXPECT_EQ(evaluate(sum, formula_cell, cells(), Settings{}), std::nullopt); +} From dccfaf4a9494a29bc5cdd0a35639a4a8f8b55bd0 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 01:07:02 +0200 Subject: [PATCH 2/3] fix: bound conditional aggregates and lookup scans --- CHANGELOG.md | 2 ++ .../formula/formula_functions_lookup.cpp | 19 ++++++++++++++++--- .../formula/formula_evaluator_test.cpp | 2 ++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1422b53f9..fcfc8dd39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ The release run heads these entries with the version and opens a fresh - Formula array limits are checked before dimension arithmetic can overflow on 32-bit targets, including broadcast results and explicit array literals. + Conditional aggregates and lookups also enforce their existing work limit + without dimension overflow. - Spreadsheet formulas keep quoted sheet spans when rewritten, read bare whole-row ranges, and report cross-sheet ranges as unresolved. Extreme diff --git a/src/odr/internal/formula/formula_functions_lookup.cpp b/src/odr/internal/formula/formula_functions_lookup.cpp index 720f32b3e..3d1a35a1e 100644 --- a/src/odr/internal/formula/formula_functions_lookup.cpp +++ b/src/odr/internal/formula/formula_functions_lookup.cpp @@ -4,6 +4,8 @@ #include #include #include +#include +#include #include #include #include @@ -40,8 +42,16 @@ class Cells final { } m_area = reference->areas.front(); whole = m_area->whole_columns || m_area->whole_rows; - columns = m_area->range.to().column - m_area->range.from().column + 1; - rows = m_area->range.to().row - m_area->range.from().row + 1; + const auto width = std::uint64_t{m_area->range.to().column} - + m_area->range.from().column + 1; + const auto height = + std::uint64_t{m_area->range.to().row} - m_area->range.from().row + 1; + if (width > std::numeric_limits::max() || + height > std::numeric_limits::max()) { + throw NoAnswer{}; + } + columns = static_cast(width); + rows = static_cast(height); } else if (const auto *matrix = std::get_if(&value.content)) { m_matrix = *matrix; columns = matrix->columns; @@ -300,7 +310,7 @@ template void each_match(const std::span conditions, const Visit &visit) { const Cells &shape = conditions.front().cells; - if (std::size_t{shape.columns} * shape.rows > cell_limit) { + if (std::uint64_t{shape.columns} * shape.rows > cell_limit) { throw NoAnswer{}; } if (std::ranges::any_of(conditions, @@ -445,6 +455,9 @@ template std::optional position_of(const Call &call, const Value &wanted, const std::uint32_t count, const bool exact, const CellAt &cell_at) { + if (count > cell_limit) { + throw NoAnswer{}; + } if (exact) { for (std::uint32_t i = 0; i < count; ++i) { if (is_lookup_match(call, wanted, cell_at(i))) { diff --git a/test/src/internal/formula/formula_evaluator_test.cpp b/test/src/internal/formula/formula_evaluator_test.cpp index 04563c874..849350e79 100644 --- a/test/src/internal/formula/formula_evaluator_test.cpp +++ b/test/src/internal/formula/formula_evaluator_test.cpp @@ -235,6 +235,8 @@ TEST(FormulaEvaluator, an_excel_comparison_near_the_15th_digit_has_no_answer) { TEST(FormulaEvaluator, array_limits_apply_before_dimension_products_narrow) { EXPECT_EQ(xlsx("=SUMPRODUCT(A1:CRXP65536)"), std::nullopt); + EXPECT_EQ(xlsx("=COUNTIF(A1:CRXP65536,1)"), std::nullopt); + EXPECT_EQ(xlsx("=MATCH(999,A1:A4294967295,0)"), std::nullopt); Node row{ArrayLiteral{65536, 1}, {}}; row.children.assign(65536, Node{NumberLiteral{1}, {}}); Node column{ArrayLiteral{1, 65536}, row.children}; From 88cb4dd3591653a76efb4735f96c222bcaa37c3e Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Mon, 5 Oct 2026 16:48:32 +0200 Subject: [PATCH 3/3] test: keep an exact match over a whole column within the lookup limit position_of now refuses a scan of more than the work limit of a lookup. A whole column fits, so an exact MATCH over A:A still finds its hit. The CHANGELOG entry is two lines. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Gb1fLafqqzehpqPuU6uBfn --- CHANGELOG.md | 6 ++---- test/src/internal/formula/formula_functions_lookup_test.cpp | 3 +++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fcfc8dd39..946b8c3d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,10 +16,8 @@ The release run heads these entries with the version and opens a fresh ## Unreleased -- Formula array limits are checked before dimension arithmetic can overflow - on 32-bit targets, including broadcast results and explicit array literals. - Conditional aggregates and lookups also enforce their existing work limit - without dimension overflow. +- Formula arrays, conditional aggregates and lookups check their size limits + before dimension arithmetic can overflow on 32-bit targets. - Spreadsheet formulas keep quoted sheet spans when rewritten, read bare whole-row ranges, and report cross-sheet ranges as unresolved. Extreme diff --git a/test/src/internal/formula/formula_functions_lookup_test.cpp b/test/src/internal/formula/formula_functions_lookup_test.cpp index 11062fb6c..facafef1b 100644 --- a/test/src/internal/formula/formula_functions_lookup_test.cpp +++ b/test/src/internal/formula/formula_functions_lookup_test.cpp @@ -150,4 +150,7 @@ TEST(FormulaFunctionsLookup, a_whole_column_past_the_extent_has_no_answer) { EXPECT_EQ(xlsx(R"(=COUNTIF(A:A,"abc"))"), number(1)); EXPECT_EQ(xlsx("=VLOOKUP(2,A:A,1)"), std::nullopt); EXPECT_EQ(xlsx("=INDEX(A:A,100)"), std::nullopt); + // an exact match ends at the first hit, within the work limit of a lookup + EXPECT_NE(xlsx(R"(=MATCH("abc",A:A,0))"), std::nullopt); + EXPECT_EQ(xlsx(R"(=MATCH("abc",A:A,0))"), xlsx(R"(=MATCH("abc",A1:A14,0))")); }