Fix use-after-free in BigInt self-addition and self-subtraction#9122
Open
tautschnig wants to merge 2 commits into
Open
Fix use-after-free in BigInt self-addition and self-subtraction#9122tautschnig wants to merge 2 commits into
tautschnig wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a use-after-free bug in CBMC’s BigInt implementation when performing self-aliased compound addition/subtraction (x += x, x -= x) by ensuring the operand digit pointer is re-derived after a potential reallocation in resize(). It also adds unit tests that reproduce the original failure mode (tight buffer from decimal-string scanning) and pins down behavior of other self-aliased operations that were already safe.
Changes:
- Fix
BigInt::add()to handle the self-aliased operand case safely acrossresize()reallocation. - Add unit tests covering
x += xandx -= xwith a string-scanned value that forces reallocation. - Add “pin-down” unit tests for
x *= x,x /= x, andx %= xself-operations.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/big-int/bigint.cc |
Re-derives aliased operand pointer after resize() to prevent UAF in self-add/self-sub. |
unit/big-int/big-int.cpp |
Adds regression and pin-down unit tests for self-aliased compound assignments and related ops. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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. Detect the self-aliased case, which is exactly dig == digit since distinct BigInt objects never share digit buffers, and re-derive the operand pointer after the resize, whose buffer replacement preserves the contents. 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 x += x and x -= x with such a tight buffer, plus pin-down tests for x *= x, x /= x and x %= x, which were already safe: multiplication computes into a fresh buffer, division and remainder take an early equal-operands exit. The compound assignments are applied through a reference, as clang's -Wself-assign-overloaded would reject some of the very patterns under test when spelled directly. Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
tautschnig
force-pushed
the
bigint-self-add
branch
from
July 20, 2026 00:09
a278d85 to
2128e97
Compare
Like add(), the private mul() method takes a raw digit buffer that aliases this->digit whenever a caller squares a number via x *= x. This is not hypothetical: pow() in bigint-func.cc squares its base via a *= a on every iteration, and the existing unit tests reach mul() through it thousands of times. That this works is down to implementation details of mul(): 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 of mul() could quietly introduce the same use-after-free just fixed in add(). Instead of relying on inspection, guard the aliased case structurally: operator*= squares via a copy of the operand, making it impossible for mul() to read freed memory no matter how it is implemented. The cost is one pointer comparison per multiplication and one buffer copy in the self-multiplication case only. Document the differing aliasing contracts on the add() and mul() declarations. Extend the self-aliased unit tests with a squaring of a value whose buffer has no spare capacity, mirroring the x += x regression test. Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #9122 +/- ##
===========================================
+ Coverage 80.77% 80.83% +0.05%
===========================================
Files 1712 1715 +3
Lines 189833 189981 +148
Branches 73 73
===========================================
+ Hits 153347 153570 +223
+ Misses 36486 36411 -75 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
Detect the self-aliased case, which is exactly dig == digit since distinct BigInt objects never share digit buffers, and re-derive the operand pointer after the resize, whose buffer replacement preserves the contents. 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 x += x and x -= x with such a tight buffer, plus pin-down tests for x *= x, x /= x and x %= x, which were already safe: multiplication computes into a fresh buffer, division and remainder take an early equal-operands exit.