From 2183ae65e87ab61fe2fcc07326de1087db906194 Mon Sep 17 00:00:00 2001 From: Michael Tautschnig Date: Wed, 22 Jul 2026 19:03:42 +0000 Subject: [PATCH 1/2] Abort on self-aliased operands in BigInt addition and subtraction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit operator+= and operator-= pass the right-hand side's digit buffer into the private add() method, which resizes *this before reading from that buffer. When the right-hand side is *this itself (x += x, x -= x) and the result requires a larger buffer, resize replaces and frees the digit array, and add() then reads the operand's digits from freed memory. Address sanitizer flags the read; the computed value may be silently wrong. Aliased operands are not supported anywhere in this library, so detect the self-aliased case — which is exactly dig == digit, since distinct BigInt objects never share digit buffers — and abort loudly instead of silently computing garbage. Callers wanting to double a number can spell it x + x or pass a copy of the operand. The bug went unnoticed because no in-tree code self-adds a BigInt, and buffers constructed via the numeric constructors carry enough spare capacity to make small self-additions stay in place; scanning from a decimal string produces a buffer without slack, making the reallocation reachable. Add unit tests for doubling and self-subtraction through a distinct copy of the operand — the supported spelling — with such a tight buffer, covering the buffer growth in add() with equal-length operands. The abort path itself is not unit-testable with Catch2. Co-authored-by: Kiro --- src/big-int/bigint.cc | 11 +++++++++++ unit/big-int/big-int.cpp | 27 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/big-int/bigint.cc b/src/big-int/bigint.cc index 978c63e2ad0..16d060b3c9d 100644 --- a/src/big-int/bigint.cc +++ b/src/big-int/bigint.cc @@ -11,6 +11,7 @@ #include #include +#include #include // How to report errors. @@ -779,6 +780,16 @@ BigInt::compare (BigInt const &b) const void BigInt::add (onedig_t const *dig, unsigned len, bool pos) { + // dig aliases this->digit when called via x += x or x -= x. The + // resize below may free that buffer, turning dig into a dangling + // pointer. Self-aliased operands are not supported; spell doubling + // as x + x or use a copy of the operand. + if(dig == digit) + { + error("BigInt::add: operand must not alias *this."); + abort(); + } + // Make sure the result fits into this, even with carry. resize ((length > len ? length : len) + 1); diff --git a/unit/big-int/big-int.cpp b/unit/big-int/big-int.cpp index 678318bf238..0e9a76492fe 100644 --- a/unit/big-int/big-int.cpp +++ b/unit/big-int/big-int.cpp @@ -75,6 +75,33 @@ TEST_CASE("arbitrary precision integers", "[core][big-int][bigint]") REQUIRE(to_string(i) == "1"); } + // ===================================================================== + // Compound assignment with an operand equal to the left-hand side. + // ===================================================================== + // Self-aliased operands (x += x) are not supported and abort at + // runtime: operator+= and operator-= pass the operand's digit buffer + // into add(), which resizes (and possibly frees) this->digit before + // reading that buffer. The tests below use a distinct copy of the + // operand, which is the supported spelling; they still exercise the + // buffer growth in add() with equal-length operands. + SECTION("compound assignment with equal operands") + { + // Scanning from a string produces a buffer without spare capacity: + // 2^192 - 1 occupies six 32-bit digits, the doubled value needs + // seven, forcing the reallocation inside add(). + BigInt x("6277101735386680763835789423207666416102355444464034512895"); + const BigInt x_copy(x); + x += x_copy; + REQUIRE( + to_string(x) == + "12554203470773361527671578846415332832204710888928069025790"); + + BigInt y("6277101735386680763835789423207666416102355444464034512895"); + const BigInt y_copy(y); + y -= y_copy; + REQUIRE(y.is_zero()); + } + // ===================================================================== // Test cases from the clisp test suite in number.tst. // ===================================================================== From 5a75a88a473f84c41f85f4ea775a7b7fbbf6f1da Mon Sep 17 00:00:00 2001 From: Michael Tautschnig Date: Wed, 22 Jul 2026 19:05:57 +0000 Subject: [PATCH 2/2] Abort on self-aliased operands in BigInt multiplication and division MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend the no-self-aliasing policy from addition and subtraction to the remaining compound-assignment operators taking a BigInt operand. mul() was safe for a self-aliased operand (x *= x) only by inspection of its current implementation: the single-digit path reads dig[0] by value before mutating, the small-this path is unreachable for equal lengths, and the general path multiplies into a fresh buffer before releasing the old one. Any restructuring could quietly introduce the same use-after-free just fixed in add(). Self-multiplication was not hypothetical: pow() squared its base via a *= a on every iteration, reaching mul() thousands of times in the existing unit tests. Square via a temporary there instead, and abort on self-aliased operands in mul() like in add(). operator/= and operator%= handle a self-aliased operand correctly today, through the value-equality fast path that serves any operands of equal magnitude. To keep the library-wide contract uniform and simple — a BigInt operand must never alias *this, no exceptions — abort there as well. Callers with operands that are merely equal in value remain unaffected. Extend the equal-operand unit tests to multiplication, division and remainder, using a distinct copy of the operand — the supported spelling. Co-authored-by: Kiro --- src/big-int/bigint-func.cc | 4 ++-- src/big-int/bigint.cc | 23 +++++++++++++++++++++++ unit/big-int/big-int.cpp | 18 ++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/big-int/bigint-func.cc b/src/big-int/bigint-func.cc index 6ee5210a5c6..fdc72562aab 100644 --- a/src/big-int/bigint-func.cc +++ b/src/big-int/bigint-func.cc @@ -20,7 +20,7 @@ pow (BigInt const &x, unsigned y) y >>= 1; if (y == 0) return r; - a *= a; + a = a * a; } } @@ -44,7 +44,7 @@ pow (BigInt const &x, BigInt const &y, BigInt const &m) b /= 2; if (b.is_zero()) return r; - a *= a; + a = a * a; } } diff --git a/src/big-int/bigint.cc b/src/big-int/bigint.cc index 16d060b3c9d..9c78c353bd5 100644 --- a/src/big-int/bigint.cc +++ b/src/big-int/bigint.cc @@ -843,6 +843,15 @@ BigInt::add (onedig_t const *dig, unsigned len, bool pos) void BigInt::mul (onedig_t const *dig, unsigned len, bool pos) { + // Self-aliased operands are not supported: parts of the code below + // read the operand after this->digit has been reallocated or + // modified. Spell squaring as x * x or use a copy of the operand. + if(dig == digit) + { + error("BigInt::mul: operand must not alias *this."); + abort(); + } + if (len < 2) { // Handle small dig/len operand efficiently. @@ -1140,6 +1149,13 @@ BigInt::div (BigInt const &x, BigInt const &y, BigInt &q, BigInt &r) BigInt & BigInt::operator/= (BigInt const &y) { + // Self-aliased operands are not supported anywhere in this library. + if(this == &y) + { + error("BigInt::operator/=: operand must not alias *this."); + abort(); + } + // Eliminate some trivial cases. int cmp = ucompare (y); if (cmp < 0) @@ -1208,6 +1224,13 @@ BigInt::operator/= (BigInt const &y) BigInt & BigInt::operator%= (BigInt const &y) { + // Self-aliased operands are not supported anywhere in this library. + if(this == &y) + { + error("BigInt::operator%=: operand must not alias *this."); + abort(); + } + // Eliminate some trivial cases. int cmp = ucompare (y); if (cmp < 0) diff --git a/unit/big-int/big-int.cpp b/unit/big-int/big-int.cpp index 0e9a76492fe..359eaad7e19 100644 --- a/unit/big-int/big-int.cpp +++ b/unit/big-int/big-int.cpp @@ -100,6 +100,24 @@ TEST_CASE("arbitrary precision integers", "[core][big-int][bigint]") const BigInt y_copy(y); y -= y_copy; REQUIRE(y.is_zero()); + + // Squaring through a copy of the operand; z *= z would abort. + BigInt z("18446744073709551616"); // 2^64 + const BigInt z_copy(z); + z *= z_copy; + REQUIRE(to_string(z) == "340282366920938463463374607431768211456"); + + // Division and remainder with equal operands take the + // value-equality fast path; w /= w and v %= v would abort. + BigInt w("340282366920938463463374607431768211456"); // 2^128 + const BigInt w_copy(w); + w /= w_copy; + REQUIRE(to_string(w) == "1"); + + BigInt v("340282366920938463463374607431768211456"); + const BigInt v_copy(v); + v %= v_copy; + REQUIRE(v.is_zero()); } // =====================================================================