Submission Details
Severity:
Duplicate User Entries Allow Winners to Lose Funds
Description
-
The
joinEvent()function should allow users to register their country choice once per deposit, adding them to theusersAddressarray one time. -
Users can call
joinEvent()multiple times with the same country, causing their address to be added tousersAddressarray multiple times. When_getWinnerShares()loops through this array, it counts the same user's shares multiple times, inflatingtotalWinnerSharesand reducing all winners' payouts.
function joinEvent(uint256 countryId) public {
// ... validation checks ...
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
usersAddress.push(msg.sender); // @> No check for duplicates - same user added multiple times
numberOfParticipants++;
totalParticipantShares += participantShares;
}
function _getWinnerShares () internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId]; // @> Counts same user multiple times
}
return totalWinnerShares;
}
Risk
Likelihood: HIGH
-
There are no access controls preventing users from calling
joinEvent()multiple times -
Users can call it repeatedly before
eventStartDatewithout any revert
Impact: HIGH
-
Winners receive significantly less funds than they deserve (proportional to duplicate entries)
-
Funds become permanently locked in the contract with no recovery mechanism
-
In test scenario: 3 duplicate entries caused 50% of vault funds to be locked forever
Proof of Concept
function test_duplicateEntriesVulnerability() public {
// User1: Deposits once, but joins event MULTIPLE times with same country
vm.startPrank(user1);
mockToken.approve(address(briVault), 5 ether);
uint256 user1shares = briVault.deposit(5 ether, user1);
briVault.joinEvent(10); // Japan - 1st time
briVault.joinEvent(10); // Japan - 2nd time
briVault.joinEvent(10); // Japan - 3rd time
vm.stopPrank();
// User2: Legitimate user - deposits and joins once
vm.startPrank(user2);
mockToken.approve(address(briVault), 5 ether);
uint256 user2shares = briVault.deposit(5 ether, user2);
briVault.joinEvent(10); // Japan - once (legitimate)
vm.stopPrank();
// User3: Another legitimate user with different country
vm.startPrank(user3);
mockToken.approve(address(briVault), 5 ether);
uint256 user3shares = briVault.deposit(5 ether, user3);
briVault.joinEvent(30); // Morocco
vm.stopPrank();
// User4: Another legitimate user with different country
vm.startPrank(user4);
mockToken.approve(address(briVault), 5 ether);
uint256 user4shares = briVault.deposit(5 ether, user4);
briVault.joinEvent(40); // South Africa
vm.stopPrank();
// Verify shares are correctly assigned
assertEq(briVault.balanceOf(user1), user1shares);
assertEq(briVault.balanceOf(user2), user2shares);
assertEq(briVault.balanceOf(user3), user3shares);
assertEq(briVault.balanceOf(user4), user4shares);
// Fast forward to after event ends
vm.warp(eventEndDate + 1);
// Owner sets Japan (country 10) as winner
vm.prank(owner);
briVault.setWinner(10);
// Calculate expected vs actual totalWinnerShares
uint256 expectedWinnerShares = user1shares + user2shares; // Only user1 and user2 picked Japan
uint256 actualWinnerShares = briVault.totalWinnerShares();
assertEq(
actualWinnerShares,
user1shares * 3 + user2shares,
"User1 counted 3 times! totalWinnerShares is inflated"
);
// Calculate impact on payouts
uint256 vaultBalance = briVault.finalizedVaultAsset();
// What winners SHOULD get (50/50 split)
uint256 user1ShouldGet = (user1shares * vaultBalance) / expectedWinnerShares;
uint256 user2ShouldGet = (user2shares * vaultBalance) / expectedWinnerShares;
// What winners WILL get (reduced payout)
uint256 user1WillGet = (user1shares * vaultBalance) / actualWinnerShares;
uint256 user2WillGet = (user2shares * vaultBalance) / actualWinnerShares;
uint256 totalLocked = (user1ShouldGet - user1WillGet) + (user2ShouldGet - user2WillGet);
// Verify both winners receive less than they should
assertLt(user1WillGet, user1ShouldGet, "User1 receives less due to inflated denominator");
assertLt(user2WillGet, user2ShouldGet, "User2 receives less due to inflated denominator");
console.log("expectedWinnerShares is: ", expectedWinnerShares);
console.log("actualWinnerShares is: ", actualWinnerShares);
console.log("user1ShouldGet is: ", user1ShouldGet);
console.log("user2ShouldGet is: ", user2ShouldGet);
console.log("user1WillGet is: ", user1WillGet);
console.log("user1WillGet is: ", user1WillGet);
}
Recommended Mitigation
Add a mapping to track whether users have already joined the event and validate against duplicate entries:
+ mapping(address => bool) public hasJoined;
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
+ if (hasJoined[msg.sender]) {
+ revert alreadyJoined();
+ }
if (countryId >= teams.length) {
revert invalidCountry();
}
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
userToCountry[msg.sender] = teams[countryId];
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
usersAddress.push(msg.sender);
+ hasJoined[msg.sender] = true;
numberOfParticipants++;
totalParticipantShares += participantShares;
}
+ function cancelParticipation() public {
// ... existing code ...
+ hasJoined[msg.sender] = false;
}
Updates
Appeal created
bube 20 days ago
Submission Judgement Published Assigned finding tags:
Duplicate registration through `joinEvent`
Rewards breakdown
nSLOC
193This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.