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

Duplicate participant entries inflate accounting, manipulate reward distribution, and open for gas DoS

fredo182

Duplicate participant entries inflate accounting, manipulate reward distribution, and open for gas DoS

Description

A user should be able to register once for a single country, ensuring each participant's shares are counted once and per-country totals remain accurate, enabling fair distribution of rewards at event finalization.

However, the joinEvent function allows the same user to call it multiple times for any or all countries, inflating accounting values and corrupting per-country reward distribution.

Risk

Likelihood: High
Any user who has deposited can repeatedly call joinEvent for one or multiple countries, as there are no checks to restrict multiple entries.

Impact: High
The impact is multi-faced:

  • Accounting Inflation: The numberOfParticipants and totalParticipantShares are artificially inflated corrupting internal metrics.

  • Reward Manipulation: Duplicate entrics overcount totalWinnerShares reducing payout per share for legitimate winners.

  • Gas DoS: Excessive repeated calls bloat usersAddress, making setWinner() potentially run out of gas.

Proof of Concept

The following test case show cases participant entries on all countries with one deposit, and the impact on contract's state.

function test_protocol_allows_duplicate_participation() public {
vm.startPrank(user1);
mockToken.approve(address(briVault), 1 ether);
briVault.deposit(1 ether, user1);
for (uint256 i = 0; i < countries.length; i++) {
briVault.joinEvent(i);
}
vm.stopPrank();
​
assertEq(briVault.balanceOf(user1), 0.985 ether);
assertEq(briVault.numberOfParticipants(), 48);
assertEq(briVault.totalParticipantShares(), 0.985 ether * 48);
for (uint256 i = 0; i < countries.length; i++) {
assertEq(briVault.usersAddress(i), user1);
assertEq(briVault.userSharesToCountry(user1, i), 0.985 ether);
}
}

Recommended Mitigation

Prevent multiple joins by the same user

+ mapping(address => bool) public hasJoined;
​
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
​
// Ensure countryId is a valid index in the `teams` array
if (countryId >= teams.length) {
revert invalidCountry();
}
​
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
​
+ if (hasJoined[msg.sender]) {
+ revert("Already joined a country");
+ }
​
+ hasJoined[msg.sender] = true;
userToCountry[msg.sender] = teams[countryId];
​
uint256 participantShares = balanceOf(msg.sender);
userSharesToCountry[msg.sender][countryId] = participantShares;
​
usersAddress.push(msg.sender);
​
numberOfParticipants++;
totalParticipantShares += participantShares;
​
emit joinedEvent(msg.sender, countryId);
}
Updates

Appeal created

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

Unbounded Loop in _getWinnerShares Causes Denial of Service

The _getWinnerShares() function is intended to iterate through all users and sum their shares for the winning country, returning the total.

Duplicate registration through `joinEvent`

Support

FAQs

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

Give us feedback!