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

Incorrect Share Calculation in joinEvent()

icis

Incorrect Share Calculation in joinEvent() Causes Over-Allocation and Payout Errors

Description

  • Expected behaviourAfter joining an event with a deposit, a user should only allocate newly acquired shares when depositing again and joining another country. When a user deposits and joins an event, if the user deposits again, the new shares gotten should be used to join any other event of their choice

  • Actual behaviour: joinEvent() uses balanceOf(msg.sender)— the total shares — every time, even if some were already allocated to prior countries. This results in over-allocation of previously used shares.

Location: joinEvent() function (line 260)

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

Risk

Likelihood: High – any user can exploit this after making multiple deposits.

Impact:

  • Users can allocate same shares to multiple countries

  • Incorrect winner share calculations

  • Winners may receive incorrect payouts

Proof of Concept

Example Attack:

  • If user deposits, joins country A, then deposits again, joins country B:

    • userSharesToCountry[user][countryA] = first deposit shares

    • userSharesToCountry[user][countryB] = total shares (includes first deposit)

    • User's shares are double-counted for country B

function test_IncorrectShareCalculation() public {
vm.startPrank(user1);
mockToken.approve(address(briVault), 20 ether);
// First deposit and join country 10
briVault.deposit(10 ether, user1);
uint256 shares1 = briVault.balanceOf(user1);
briVault.joinEvent(10);
uint256 allocatedToCountry10_1 = briVault.userSharesToCountry(user1, 10);
console.log("After first deposit - total shares:", shares1);
console.log("Allocated to country 10:", allocatedToCountry10_1);
// Second deposit (same user)
briVault.deposit(10 ether, user1);
uint256 shares2 = briVault.balanceOf(user1);
console.log("After second deposit - total shares:", shares2);
// Join different country
briVault.joinEvent(20);
​
uint256 allocatedToCountry10 = briVault.userSharesToCountry(user1, 10);
uint256 allocatedToCountry20 = briVault.userSharesToCountry(user1, 20);
console.log("Allocated to country 10 (after second join):", allocatedToCountry10);// 9.85 shares
console.log("Allocated to country 20:", allocatedToCountry20);// 19.7 shares.
// Issue: country 20 gets ALL shares (including first deposit)
// But country 10 still has the old value
// if the second country should win the shares calculation would be inflated for this user
assertEq(allocatedToCountry10, shares1, "Country 10 still has old shares");
assertEq(allocatedToCountry20, shares2, "Country 20 gets all shares");
​
vm.stopPrank();
}

Recommended Mitigation

Add this piece of code to the joinEvent() function to track incremental shares per country for a user.

// Track incremental shares per country
uint256 currentShares = balanceOf(msg.sender);
uint256 previousAllocatedShares = 0;
for (uint256 i = 0; i < teams.length; ++i) {
previousAllocatedShares += userSharesToCountry[msg.sender][i];
}
uint256 newShares = currentShares - previousAllocatedShares;
userSharesToCountry[msg.sender][countryId] += newShares;
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!