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

`BriVault::joinEvent` allows multiple team joins with the same balance, inflating totalWinnerShares.

jfornells

BriVault::joinEvent allows multiple team joins with the same balance, inflating totalWinnerShares.

Description

  • Each user should assign their shares to a single team, so their balance cannot be reused to credit multiple teams.

  • joinEvent() does not prevent re-entries. Each call overwrites userToCountry[msg.sender] with the latest team, writes userSharesToCountry[msg.sender][countryId] = balanceOf(msg.sender) for every team joined, and pushes msg.sender to usersAddress every time. As a result, the denominator totalWinnerShares can be inflated with duplicates; if the user's last team is not the winner, they cannot withdraw, but they have inflated the denominator used in:

function joinEvent(uint256 countryId) public {
​
(... )
​
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
​
@> // MISSING validation: limit to a single join per user
// and prevent reusing the same balance/shares for multiple teams.
​
userToCountry[msg.sender] = teams[countryId];
​
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
​
usersAddress.push(msg.sender);
​
(... )
}

Risk

Likelihood: High

  • It is trivial to loop joinEvent() before the tournament starts, betting on all countries as many times as desired with the same balance.

Impact: High

  • When the owner sets the winner, _getWinnerShares() sums duplicate entries and inflates totalWinnerShares. This causes legitimate winners to receive less than they should and leaves leftover tokens locked in the contract.

Proof of Concept

The test shows that a user can call joinEvent() multiple times with the same balance, inflating totalWinnerShares.
This causes legitimate winners to receive less in withdraw(), while the attacker cannot withdraw because their last team is not the winner.

function test_MultiJoin_InflatesTotalWinnerShares_And_BlocksWithdraw() public {
// --- setup: user1 (attacker) and user2 (legitimate winner) ---
vm.startPrank(user1);
mockToken.approve(address(briVault), 10 ether);
briVault.deposit(10 ether, user1);
uint256 s1 = briVault.balanceOf(user1);
vm.stopPrank();
​
vm.startPrank(user2);
mockToken.approve(address(briVault), 10 ether);
briVault.deposit(10 ether, user2);
uint256 s2 = briVault.balanceOf(user2);
vm.stopPrank();
​
// --- user1 reuses the same balance to join all teams ---
vm.startPrank(user1);
for (uint256 i; i < 48; i++) briVault.joinEvent(i); // last one is NOT the winner
vm.stopPrank();
​
// user2 bets only on the winner (e.g. 10)
vm.prank(user2);
briVault.joinEvent(10);
​
// --- set winner and check inflation ---
vm.warp(briVault.eventEndDate() + 1);
vm.prank(owner);
briVault.setWinner(10);
​
// totalWinnerShares = s2 + (s1 * number of times user1 appears in usersAddress with winnerId)
// since user1 did 48 joins and one of them is 10, it's counted 48 times
assertEq(briVault.totalWinnerShares(), s2 + (s1 * 48), "denominator must be inflated");
​
// --- attacker cannot withdraw (last team is not the winner) ---
vm.startPrank(user1);
vm.expectRevert(BriVault.didNotWin.selector);
briVault.withdraw();
vm.stopPrank();
​
// --- (optional and short) legitimate winner receives less due to inflation ---
uint256 balBefore = mockToken.balanceOf(user2);
vm.prank(user2);
briVault.withdraw();
uint256 received = mockToken.balanceOf(user2) - balBefore;
​
// payout with inflation < payout without inflation
uint256 vaultAsset = briVault.finalizedVaultAsset();
uint256 expectedNoInflation = (s2 * vaultAsset) / (s1 + s2);
assertLt(received, expectedNoInflation, "payout diluted by inflated denominator");
}
​
Ran 1 test for test/briVault.t.sol:BriVaultTest
[PASS] test_MultiJoin_InflatesTotalWinnerShares_And_BlocksWithdraw() (gas: 3541844)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 25.02ms (7.66ms CPU time)

Recommended Mitigation

Add a mapping to track users who have already joined.

​
------- ERRORS ------
+ error AlreadyJoined();
​
​
------- STORAGE ------
+ mapping(address => bool) public joined;
​
​
function joinEvent(uint256 countryId) public {
(... )
​
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
​
+ if (joined[msg.sender]) revert AlreadyJoined(); // prevent multiple joins
​
​
userToCountry[msg.sender] = teams[countryId];
​
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
​
+ joined[msg.sender] = true;
usersAddress.push(msg.sender);
​
numberOfParticipants++;
(... )
}
Updates

Appeal created

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

Duplicate registration through `joinEvent`

Support

FAQs

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

Give us feedback!