BriVault
First Flight #52
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: medium
Invalid

Double Counting in _getWinnerShares()

codeaudit0x1

Root + Impact

Description

  • The normal behavior of _getWinnerShares() is to calculate the total shares held by all participants who selected the winning country.

  • However, the current implementation does not reset totalWinnerShares before summing, which causes the value to accumulate repeatedly on multiple calls instead of reflecting the true total of winner shares.

This results in incorrect payout calculations during withdrawals since the contract will believe there are more winning shares than actually exist.

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

Risk

Likelihood:

  • Very likely — _getWinnerShares() is called from setWinner(), but it can also be called again internally or in future code updates.

  • Any subsequent call after the first will double-count user shares since the state variable totalWinnerShares keeps accumulating without being cleared.

Impact:

  • Incorrect reward distribution — winners will receive less assets than expected because the denominator (totalWinnerShares) becomes inflated.

  • Permanent corruption of vault accounting — since totalWinnerShares is a persistent state variable, wrong values will persist across function calls and affect all future reward logic.

Proof of Concept

// Assume two users each hold 100 shares in the winning country.
totalWinnerShares = 0;
_getWinnerShares(); // totalWinnerShares = 200
_getWinnerShares(); // totalWinnerShares = 400 (incorrectly doubled)
_getWinnerShares(); // totalWinnerShares = 600 (keeps increasing)
​
// Withdraw calculations now use inflated denominator -> users receive less
​

Recommended Mitigation

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

  • Reset totalWinnerShares to zero before summing user shares.

  • Optionally, mark the function as view and compute totals on-demand if state persistence is not required.

Updates

Appeal created

bube Lead Judge 20 days ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!