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

Non-winner entries can cause loss in rewards to the winners

hunterspartan5

Root + Impact

Description

  • In joinEvent(); function, any participant who has alraedy deposited assets can call this function multiple times causing multiple entries in the userSharesToCountry state variable (as there are 48 countries, participant can create entries for all of them in this state variable). So at the end of the betting round, when the owner sets the winner, _getWinnerShares(); function is called which updates the totalWinnerShares state variable. totalWinnerShares is used to calculate the percentage ownership of the current winner in withdraw(); function and accordingly the winner gets his reward. (totalWinnerShares - also count such dummy entires causing lose in the rewards amount to the users)

// Root cause in the codebase with @> marks to highlight the relevant section
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
​
if (countryId >= teams.length) {
revert invalidCountry();
}
​
if (block.timestamp > eventStartDate) {
revert eventStarted();
}
​
userToCountry[msg.sender] = teams[countryId];
​
uint256 participantShares = balanceOf(msg.sender);
// This causes harm to the winners as there are non winner entries too that are considered
// while getting the prize - high
userSharesToCountry[msg.sender][countryId] = participantShares;
​
usersAddress.push(msg.sender);
​
numberOfParticipants++;
totalParticipantShares += participantShares;
​
emit joinedEvent(msg.sender, countryId);
}

Risk

Likelihood:

  • When the participant has already deposited his assets using the deposit(); function, he can call joinEvent any number of times (countryId is also used to update, and there are 48 counties only so apparently 48 times).

Impact:

  • Loss in reward amount to the actual winners.

Proof of Concept

​

Recommended Mitigation

  • Refactoring of the code is required such that, the last countryId should be taken into consideration. There should be only one entry in the userSharesToCountry state variable and for only one countryId.

- remove this code
+ add this code
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!