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

Stale per-country share snapshots after top-ups lock winner withdrawals

galer ah

Root + Impact

Failing to refresh userSharesToCountry after additional deposits lets winners claim more than the vault balance, causing withdrawals to revert and effectively locking their rewards (High).

Description

When a user joins an event, the contract snapshots their current share balance and stores it under the selected country:

function joinEvent(uint256 countryId) public {
...
uint256 participantShares = balanceOf(msg.sender);
​
@> userSharesToCountry[msg.sender][countryId] = participantShares;
​
usersAddress.push(msg.sender);
...
}

If the participant later deposits more tokens, their real share balance (balanceOf(msg.sender)) increases, but userSharesToCountry[msg.sender][countryId] remains stale. When the winner is set, _getWinnerShares() computes the denominator for reward distribution using this outdated snapshot:

function _getWinnerShares() internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i) {
address user = usersAddress[i];
​
@> totalWinnerShares += userSharesToCountry[user][winnerCountryId];
​
}
return totalWinnerShares;
}

During withdraw(), the numerator uses the current (larger) balanceOf(msg.sender), while the denominator is based on the stale snapshot. If the discrepancy is large enough, Math.mulDiv asks the vault to transfer more tokens than it actually holds, causing the ERC20 transfer to revert:

function withdraw() external winnerSet {
...
uint256 shares = balanceOf(msg.sender);
uint256 vaultAsset = finalizedVaultAsset;
​
@> uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);
​
_burn(msg.sender, shares);
@> IERC20(asset()).safeTransfer(msg.sender, assetToWithdraw);
}

Risk

Any honest participant who deposits again after calling joinEvent (and either cannot or does not re-join before eventStartDate) risks hitting a revert when withdrawing after their country wins. No privileged access is needed; this is a realistic loss-of-funds scenario driven by normal user behavior.

Impact:

  • Winning users who deposit multiple times cannot withdraw their rewards.

  • Vault funds remain locked, breaking the payout process for affected winners.

Proof of Concept

  1. Alice deposits 100 tokens and calls joinEvent(0).

  2. Still before eventStartDate, she deposits another 900 tokens but does not (or cannot) call joinEvent again.

  3. Alice’s team wins; the owner calls setWinner(0).

  4. Alice calls withdraw(). The function attempts to transfer more tokens than exist in the vault and the transaction reverts.

function test_withdrawRevertsAfterTopUp() public {
vm.startPrank(alice);
vault.deposit(100e18, alice);
vault.joinEvent(0);
vault.deposit(900e18, alice);
vm.stopPrank();
​
vm.warp(eventEndDate + 1);
vm.prank(owner);
vault.setWinner(0);
​
vm.prank(alice);
vm.expectRevert();
vault.withdraw();
}

Recommended Mitigation

  1. Track incremental share contributions per country instead of overwriting with balanceOf(msg.sender):

- uint256 participantShares = balanceOf(msg.sender);
- userSharesToCountry[msg.sender][countryId] = participantShares;
+ uint256 newShares = balanceOf(msg.sender) - userSharesToCountry[msg.sender][countryId];
+ userSharesToCountry[msg.sender][countryId] += newShares;

  1. Alternatively, move the per-country share accounting into deposit so each mint updates the correct bucket in real time.

  2. Add tests covering multiple deposits between joinEvent and setWinner to ensure withdrawals remain functional.

Updates

Appeal created

bube Lead Judge 16 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
bube Lead Judge 12 days ago
Submission Judgement Published
Validated
Assigned finding tags:

Deposits after joining the event are not correctly accounted

Support

FAQs

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

Give us feedback!