First Flight #13: Baba Marta
First Flight #13
Beginner FriendlyFoundryGameFi
100 EXP
View results
Previous Next
Submission Details
Severity: low
Invalid

Looping through `MartenitsaVoting::_tokenIds` storage array in `MartenitsaVoting::announceWinner` function can lead to denial of service to announce the winner.

mirkopezo

Summary

For each vote MartenitsaVoting::_tokenIds arrays grows in size. _tokenIds array can grow so big that it will not be possible to announce the winner by calling MartenitsaVoting::announceWinner function.

Vulnerability Details

function announceWinner() external onlyOwner {
require(block.timestamp >= startVoteTime + duration, "The voting is active");
​
uint256 winnerTokenId;
uint256 maxVotes = 0;
​
@> for (uint256 i = 0; i < _tokenIds.length; i++) {
if (voteCounts[_tokenIds[i]] > maxVotes) {
maxVotes = voteCounts[_tokenIds[i]];
winnerTokenId = _tokenIds[i];
}
}
​
list = _martenitsaMarketplace.getListing(winnerTokenId);
_healthToken.distributeHealthToken(list.seller, 1);
​
emit WinnerAnnounced(winnerTokenId, list.seller);
}

Impact

If MartenitsaVoting::_tokenIds grows big, it will never be possible to announce the winner. In that scenario it means that protocol is basically unusable.

Proof of Concept

Place the following test into MartenitsaVoting.t.sol. Import console from forge-std/Test.sol.

As MartenitsaVoting::_tokenIds array grows, it will cost more gas to announce the winner.

function testAnnounceWinnerDoS() public listMartenitsa {
vm.txGasPrice(1);
​
voting.startVoting();
​
for (uint256 i; i < 100; i++) {
vm.prank(address(uint160(i)));
voting.voteForMartenitsa(0);
}
​
vm.warp(block.timestamp + 1 days + 1);
uint256 gasBefore = gasleft();
voting.announceWinner();
uint256 gasAfter = gasleft();
uint256 gasTotal = gasBefore - gasAfter;
console.log("Gas cost to announce winner with first 100 votes: %s", gasTotal);
​
voting.startVoting();
​
for (uint256 i = 100; i < 200; i++) {
vm.prank(address(uint160(i)));
voting.voteForMartenitsa(0);
}
​
vm.warp(block.timestamp + 1 days + 1);
uint256 gasBefore2 = gasleft();
voting.announceWinner();
uint256 gasAfter2 = gasleft();
uint256 gasTotal2 = gasBefore2 - gasAfter2;
console.log("Gas cost to announce winner with second 100 votes: %s", gasTotal2);
​
assert(gasTotal2 > gasTotal);
}

Test output:

Gas cost to announce winner with first 100 votes: 129275
Gas cost to announce winner with second 100 votes: 139675

Tools Used

Manual review

Recommendations

Change logic of MartenitsaVoting::announceWinner function so there is no looping through dynamic storage array.

Updates

Lead Judging Commences

bube Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Unbounded arrays

mirkopezo Submitter
over 1 year ago
bube Lead Judge
over 1 year ago
bube Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Unbounded arrays

Support

FAQs

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