MartenitsaVoting::announceWinner
function does not check if there are no votes, causing a participant to become a winner and receive a HealthTokenLow Risk
https://github.com/Cyfrin/2024-04-Baba-Marta/blob/5eaab7b51774d1083b926bf5ef116732c5a35cfd/src/MartenitsaVoting.sol#L57
Users can vote for the best MartenitsaToken
NFT, which is listed for sale on the MartenitsaMarketplace
. The MartenitsaVoting::announceWinner
function is used to calculate the winner of the voting event. The winner is the NFT with the most votes. However, the MartenitsaVoting::announceWinner
function does not check if there are no votes, which a participant can receive a HealthToken
even if there are no votes for the winner's NFT, and become the winner.
A participant can become a winner of the event and receive a HealthToken
even if there are no votes for the winner's NFT, leading to unfair results in voting events.
Manual Review
Add this test to MartenitsaVoting.t.sol
:
function testIfThereAreNoVotes() public {
// Chasy lists a martenitsa for sale
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet");
marketplace.listMartenitsaForSale(0, 1 wei);
vm.stopPrank();
// Jack lists a martenitsa for sale
vm.startPrank(jack);
martenitsaToken.createMartenitsa("bracelet");
marketplace.listMartenitsaForSale(1, 1 wei);
vm.stopPrank();
// Announcing winner
vm.warp(block.timestamp + 1 days + 1);
vm.recordLogs();
voting.announceWinner();
console.log("Chasy's Health Token balance: ", healthToken.balanceOf(chasy));
console.log("Jack's Health Token balance: ", healthToken.balanceOf(jack));
Vm.Log[] memory entries = vm.getRecordedLogs();
address winner = address(uint160(uint256(entries[0].topics[2])));
assert(winner == chasy);
}
As we can see from the logs of this test the HealthToken
balance of Chasy is 1. Jack's HealthToken
balance is 0, because he is not the winner.
Consider adding a require
statement to check if maxVotes
, which is declared in Martenitsa::announceWinner
function, is greater than 0:
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];
}
}
+ require(maxVotes > 0, "There are no votes");
list = _martenitsaMarketplace.getListing(winnerTokenId);
_healthToken.distributeHealthToken(list.seller, 1);
emit WinnerAnnounced(winnerTokenId, list.seller);
}