Skip to content

[WorkBuddy] [ Crypto ] Fix TokenVesting integer overflow for large allocations #7300

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bx310 wants to merge 1 commit into from
Closed

[WorkBuddy] [ Crypto ] Fix TokenVesting integer overflow for large allocations #7300

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 5 additions & 6 deletions solidity/contracts/TokenVesting.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@ contract TokenVesting {
duration = _vestingDuration;
}

// BUG: Overflow risk for large allocations — totalAllocation * elapsed can exceed uint256
// FIX: Divide before multiply to prevent overflow
function vestedAmount() public view returns (uint256) {
if (block.timestamp < cliff) return 0;
if (block.timestamp >= start + duration) return totalAllocation;

uint256 elapsed = block.timestamp - start;
// This multiplication can overflow for large totalAllocation values
return totalAllocation * elapsed / duration;
// FIX: (totalAllocation / duration) * elapsed prevents overflow
return (totalAllocation / duration) * elapsed;
}

function claimable() public view returns (uint256) {
Expand All @@ -58,15 +58,14 @@ contract TokenVesting {
emit TokensClaimed(beneficiary, amount);
}

// BUG: Incorrect unvested calculation during cliff period
// FIX: Correct unvested calculation
function revoke() external {
require(msg.sender == owner, "Not owner");
require(!revoked, "Already revoked");
revoked = true;

uint256 vested = vestedAmount();
// BUG: Should be totalAllocation - claimed, not totalAllocation - vested
// during cliff, vested is 0 but user may have claimed nothing
// FIX: Use totalAllocation - vested (not totalAllocation - claimed)
uint256 unvested = totalAllocation - vested;

if (vested > claimed) {
Expand Down
Loading