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

DOS Attack via Unbounded usersAddress Array Growth

objectplayer

Root + Impact

The usersAddress array can grow unbounded with unlimited and even duplicate entries, causing gas exhaustion and locking all funds.

Description

  • The usersAddress array should contain unique addresses of participants and limited participants

  • The joinEvent() function pushes users without any upper limit check and even without checking for duplicates.

  • Users can call it multiple times or rejoin after canceling, filling the array with duplicates.

function joinEvent(uint256 countryId) public {
// ... validation ...
usersAddress.push(msg.sender); // @> No duplicate check
numberOfParticipants++;
}
​
function _getWinnerShares () internal returns (uint256) {
for (uint256 i = 0; i < usersAddress.length; ++i){ // @> Unbounded loop
address user = usersAddress[i];
totalWinnerShares += userSharesToCountry[user][winnerCountryId];
}
return totalWinnerShares;
}

Risk

Likelihood:

  • Users can join multiple times with different countries

  • Cancel and rejoin creates duplicates

  • Single attacker can execute this easily

Impact:

  • setWinner() runs out of gas iterating large array

  • Winner cannot be set

  • All funds permanently locked

  • Complete protocol DOS

Proof of Concept

Here is the PoC for calling joinEvent() multiple times to increase the size of the arrray

// Attacker joins repeatedly
attacker.deposit(minimumAmount, attacker);
​
for (uint i = 0; i < 1000; i++) {
attacker.joinEvent(i % 48);
// usersAddress contains attacker 1000 times
}
​
// Owner tries to set winner
owner.setWinner(0);
// REVERT: Out of gas in _getWinnerShares()
​
// All funds locked forever

Recommended Mitigation

To fix the duplication add hasJoined mapping and add an upper bound to the user array to limit the participants

+ mapping(address => bool) public hasJoined;
+ uint256 public maxParticipants; // 100 or 1000 depending on the execution
​
function joinEvent(uint256 countryId) public {
if (stakedAsset[msg.sender] == 0) {
revert noDeposit();
}
+ if (hasJoined[msg.sender]) {
+ revert("Already joined");
+ }
​
// ... rest of logic ...
usersAddress.push(msg.sender);
+ hasJoined[msg.sender] = true;
}
​
function cancelParticipation () public {
// ... existing logic ...
+ hasJoined[msg.sender] = false;
}
Updates

Appeal created

bube Lead Judge 19 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.

Support

FAQs

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

Give us feedback!