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

Incorrect Winner Announcement Handling in announceWinner Function

pelz

Summary

The announceWinner function in the MartenitsaVoting.sol contract does not handle tie scenarios properly. In cases where multiple tokens receive the same maximum number of votes, only the first token encountered is announced as the winner, neglecting other tokens with the same vote count.

Vulnerability Details

The vulnerability lies in the logic of the announceWinner function, which does not handle tie scenarios properly. In cases where multiple tokens receive the same maximum number of votes, only the first token encountered is announced as the winner, neglecting other tokens with the same vote count.

Proof of Concept (POC) to be added to MartenitsaVoting.t.sol:

function testVoteForMartenitsaTwice() public listMartenitsa {
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet2");
marketplace.listMartenitsaForSale(1, 1 wei);
vm.stopPrank();
​
vm.prank(address(uint160(1)));
voting.voteForMartenitsa(0);
​
vm.prank(address(uint160(2)));
voting.voteForMartenitsa(1);
​
assert(voting.voteCounts(0) == 1);
assert(voting.voteCounts(1) == 1);
}
​
function testAnnounceWinnerWrongly() public {
testVoteForMartenitsaTwice();
vm.warp(block.timestamp + 1 days + 1);
vm.recordLogs();
voting.announceWinner();
​
Vm.Log[] memory entries = vm.getRecordedLogs();
address winner = address(uint160(uint256(entries[0].topics[2])));
assert(winner == chasy);
}

Impact

This vulnerability can lead to incorrect determination of winners in tie scenarios, resulting in unfair outcomes and potentially undermining the integrity and credibility of the voting process. It may also impact user trust in the platform and the reliability of voting results.

Tools Used

manual code review.

Recommendations

To address this vulnerability and ensure fair determination of winners in tie scenarios, it is recommended to modify the announceWinner function to handle tie situations appropriately. Consider implementing logic to identify and announce all tied tokens as winners if they share the maximum vote count.

Example:

function announceWinner() external onlyOwner {
require(block.timestamp >= startVoteTime + duration, "The voting is active");
​
uint256 maxVotes = 0;
address[] memory winners;
​
// Find the maximum number of votes
for (uint256 i = 0; i < _tokenIds.length; i++) {
if (voteCounts[_tokenIds[i]] > maxVotes) {
maxVotes = voteCounts[_tokenIds[i]];
}
}
​
// Identify all tokens with the maximum vote count as winners
for (uint256 i = 0; i < _tokenIds.length; i++) {
if (voteCounts[_tokenIds[i]] == maxVotes) {
winners.push(_tokenIds[i]);
}
}
​
// Distribute rewards to all winners
for (uint256 i = 0; i < winners.length; i++) {
list = _martenitsaMarketplace.getListing(winners[i]);
_healthToken.distributeHealthToken(list.seller, 1);
emit WinnerAnnounced(winners[i], list.seller);
}
}
Updates

Lead Judging Commences

bube Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Tie in voting is not considered

Support

FAQs

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