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

If the Vote ends in a draw then the `MartenitsaVoting::announceWinner` picks the first producer to get highest votes as the winner instead of both

codexbugmenot

Summary

There is no condition in MartenitsaVoting::announceWinner which checks for the case of vote resulting in a draw which then picks only the first producer to get highest votes as a winner

Impact

If the vote ends in a draw then the MartenitsaVoting::announceWinner picks the first producer to get highest votes as the winner where as both players have won the voting.

Proof of Concept

Note: Please Import {console} in MartenitsaVoting.t.sol by adding import {console} from "forge-std/Test.sol"; at the top for the PoC's to work effortlessly.

Voting results in a draw but the first producer to get highest votes is announced as winner

PoC: Announce Winner Draw
function testAnnounceWinnerDraw() public startVoting {
vm.startPrank(jack);
martenitsaToken.createMartenitsa("bracelet");
marketplace.listMartenitsaForSale(0, 1 wei);
vm.stopPrank();
​
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet");
marketplace.listMartenitsaForSale(1, 1 wei);
vm.stopPrank();
​
// changing bobs vote to 1 and jacks vote to 0 changes the winner
vm.prank(bob);
voting.voteForMartenitsa(0);
​
vm.prank(jack);
voting.voteForMartenitsa(1);
​
console.log("Vote Count 0 :", voting.getVoteCount(0));
console.log("Vote Count 1 :", voting.getVoteCount(1));
​
vm.warp(block.timestamp + 1 days);
​
address owner = address(0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496);
vm.prank(owner);
voting.announceWinner();
}
​

Recommendations

  1. Votes May result in a draw hence instead of using uint256 winnerTokenId; using a uint256 array i.e uint256[] winnerTokenId; to push the winners
    and announcing everyone on the array as a winner is a better choice.

Make the following Changes in MartenitsaVoting::announceWinner function:

- uint256 winnerTokenId;
+ uint256[] winnerTokenId;
​
​
- if (voteCounts[_tokenIds[i]] > maxVotes)
+ if (voteCounts[_tokenIds[i]] >= maxVotes)
​
​
- winnerTokenId = _tokenIds[i];
+ winnerTokenId.push(_tokenIds[i]);
- list = _martenitsaMarketplace.getListing(winnerTokenId);
- _healthToken.distributeHealthToken(list.seller, 1);
- emit WinnerAnnounced(winnerTokenId, list.seller);
​
+ for(uint256 i=0;i<winnerTokenId.length;i++){
+ list = _martenitsaMarketplace.getListing(winnerTokenId[i]);
+ _healthToken.distributeHealthToken(list.seller, 1);
+ emit WinnerAnnounced(winnerTokenId[i], list.seller);
+ }
​

  1. Alternatively, if only one winner is to be picked then owner can hold his vote until the end to solve the draw by voting to any one of the tokenIds , relevant changed should be done in the code to not announce the winner in case of draw and let owner make the final vote .

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.