fix: prevent VestingWallet bricking from uint256 overflow in vestedAmount

[DeVikingMark]

Summary

Fixes arithmetic overflow vulnerability in VestingWallet.vestedAmount() that could brick the contract when balance + released exceeds type(uint256).max.

Changes

• Add overflow protection in vestedAmount() functions for both ETH and ERC20 tokens
• Cap total allocation at type(uint256).max when overflow would occur
• Fix multiplication overflow in _vestingSchedule() with alternative calculation method
• Handle edge case where timeElapsed = 0 to prevent division by zero

Fixes #5793

[changeset-bot]

⚠️ No Changeset found

Latest commit: 47d73d1

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[coderabbitai]

Walkthrough

• Updated VestingWallet.sol to use saturating addition when computing total vested base amounts:
  • ETH: address(this).balance saturatingAdd released()
  • ERC20: IERC20(token).balanceOf(address(this)) saturatingAdd released(token)
• Replaced linear vesting math (totalAllocation * (timestamp - start()) / duration()) with mulDiv(totalAllocation, timestamp - start(), duration()).
• Changes applied to both ETH and token vesting calculations.
• Aims to prevent overflows from aggregate inbound transfers exceeding type(uint256).max, as described in linked issue VestingWallet bricked if aggregate of transfers in exceed type(uint256).max #5793.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title succinctly and accurately captures the primary change: preventing VestingWallet from being bricked by a uint256 overflow in vestedAmount and matches the PR's described fixes to VestingWallet.vestedAmount. It is concise, specific, and appropriate for a single-line PR title.
Linked Issues Check	✅ Passed	The changes in the summary—replacing unsafe addition with a saturating add/cap at type(uint256).max and switching to a mulDiv-based vesting calculation to avoid multiplication overflow—directly address the bricking scenario described in issue #5793 by preventing balance+released overflow and fixing vesting math. These code-level fixes target the exact failure mode reproduced in the issue and therefore satisfy the primary coding objectives.
Out of Scope Changes Check	✅ Passed	All modifications described in the raw summary are limited to VestingWallet's vestedAmount and vesting calculation and align with the linked issue's scope; there is no indication of unrelated files or functionality being changed. The changes appear focused and in-scope.
Description Check	✅ Passed	The description directly describes the changes summarized in the raw summary: adding overflow protection in vestedAmount for ETH and ERC20, capping total allocation at type(uint256).max, using a safer vesting calculation to avoid multiplication overflow, and handling the timeElapsed edge case, and it references the linked issue #5793. The description is on-topic and meets the lenient requirement for this check.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b9c7078 and 47d73d1.

📒 Files selected for processing (1)

• contracts/finance/VestingWallet.sol (2 hunks)

🧰 Additional context used

🪛 GitHub Actions: formal verification

contracts/finance/VestingWallet.sol

[error] 136-136: Compiler error: Member "saturatingAdd" not found or not visible after argument-dependent lookup in uint256. This occurred during 'forge build' while compiling VestingWallet.sol at line 136.

🪛 GitHub Actions: checks

contracts/finance/VestingWallet.sol

[error] 187-187: Hardhat compilation failed during coverage. TypeError: saturatingAdd not found or not visible after argument-dependent lookup in uint256 at VestingWallet.sol:187.

🔇 Additional comments (3)

contracts/finance/VestingWallet.sol (3)

136-136: Saturating total allocation prevents overflow bricking.

Good change; with saturation, releasable() remains ≤ balance in overflow scenarios.

Please add/extend tests that simulate balance + released > type(uint256).max for both ETH and ERC20 to ensure releasable() stays callable and releases a non-reverting amount.

---

143-143: Token path parity maintained.

Mirroring ETH logic for ERC20 with saturatingAdd is correct and consistent.

---

156-156: Use of mulDiv avoids intermediate overflow.

Safer than totalAllocation * time / duration. Branches already guard duration()==0, so no div-by-zero risk.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}