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

Array 'usersAddress' makes incorrect calculation of winner payouts in some cases

al88nsk

Description

There are 2 issues with array usersAddress in contract BriVault:

  1. It is node changed when user cancels participation

  2. If user calls join several times, the array will contain several occurrences of the user's address. This can happen if user joins then cancels and again joins, or joins twice accidentally

The array affects calculation of totalWinnerShares:

function _getWinnerShares () internal returns (uint256) {
​
for (uint256 i = 0; i < usersAddress.length; ++i){
address user = usersAddress[i];
@> totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
return totalWinnerShares;
}

And then totalWinnerShares is used to calculate payouts:

uint256 assetToWithdraw = Math.mulDiv(shares, vaultAsset, totalWinnerShares);

So, winners get incorrect amount of assets in cases described above.

Risk

Likelihood:

High, since the situations when the issue occurs are quite possible.

Impact:

High, since winners get less assets than they must get.

Recommended Mitigation

  1. Delete the user from the array when user cancels participation

  2. Revert if users try to join when the user is joined already

Updates

Appeal created

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

`cancelParticipation` Leaves Stale Winner Data

CancelParticipation burns shares but leaves the address inside usersAddress and keeps userSharesToCountry populated.

Duplicate registration through `joinEvent`

Support

FAQs

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

Give us feedback!