Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh

## Unreleased

- 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
reference shifts give `#REF!` without signed overflow.
Expand Down
42 changes: 25 additions & 17 deletions src/odr/internal/formula/formula_evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::size_t>(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;
Expand Down Expand Up @@ -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<Matrix>(&value.content)) {
Expand All @@ -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<std::uint32_t>(columns),
static_cast<std::uint32_t>(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(
Expand Down Expand Up @@ -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)));
}
Expand Down Expand Up @@ -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),
Expand Down
19 changes: 16 additions & 3 deletions src/odr/internal/formula/formula_functions_lookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#include <algorithm>
#include <array>
#include <cmath>
#include <cstdint>
#include <limits>
#include <optional>
#include <span>
#include <string>
Expand Down Expand Up @@ -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<std::uint32_t>::max() ||
height > std::numeric_limits<std::uint32_t>::max()) {
throw NoAnswer{};
}
columns = static_cast<std::uint32_t>(width);
rows = static_cast<std::uint32_t>(height);
} else if (const auto *matrix = std::get_if<Matrix>(&value.content)) {
m_matrix = *matrix;
columns = matrix->columns;
Expand Down Expand Up @@ -300,7 +310,7 @@ template <typename Visit>
void each_match(const std::span<const Condition> 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,
Expand Down Expand Up @@ -445,6 +455,9 @@ template <typename CellAt>
std::optional<std::uint32_t>
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))) {
Expand Down
13 changes: 13 additions & 0 deletions test/src/internal/formula/formula_evaluator_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include <optional>
#include <string>
#include <utility>

using namespace odr;
using namespace odr::internal;
Expand Down Expand Up @@ -231,3 +232,15 @@ 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);
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};
const Node sum{BinaryOperation{BinaryOperator::add},
{std::move(row), std::move(column)}};
EXPECT_EQ(evaluate(sum, formula_cell, cells(), Settings{}), std::nullopt);
}
3 changes: 3 additions & 0 deletions test/src/internal/formula/formula_functions_lookup_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))"));
}
Loading