Summary
MartenitsaVoting:announceWinner don't manage if there is a tie at the end of the voting period, if there is a tie, the winner will be the one with the lowest tokenID
Vulnerability Details
In MartenitsaVoting:announceWinner the for loop iterate on the length of _tokenIDs array and never check if MartenitsaTokenId mapping has an equality regarding the number of votes.
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];
}
}
Impact
In the case of a tie between participants, selection of the winner is unfair and can lead to user disinterest in the voting system
Tools Used
Manuel review
Recommendations
MartenitsaVoting:announceWinner must be refactored and the best way to get a real randomness for the draw consider to use chainlink VFR
import "@chainlink/contracts/src/v0.8/VRFConsumerBase.sol";
Add VRFConsumerBase as inheritance
contract VotingContract is VRFConsumerBase, Ownnable {...}
Modifiy the constructor and add some news state variables
bytes32 internal keyHash;
uint256 internal fee;
uint256[] private potentialWinners;
constructor(address marketplace, address healthToken, address _martenitsaToken)
Ownable(msg.sender)
VRFConsumerBase(
0x8FbB18354d37f7A587C60f1364b8aA2a05f69B90,
0x326C977E6efc84E512bB9C30f76E30c160eD06FB
)
{
_martenitsaMarketplace = MartenitsaMarketplace(marketplace);
_healthToken = HealthToken(healthToken);
martenitsaToken = IMartenitsaToken(_martenitsaToken);
keyHash = 0x0476f9e5d797756e0f243a643e406a30e46e83c480a91cd96a1769f9c22daa60;
fee = 0.1 * 10 ** 18;
owner = msg.sender;
}
Refactor MartenitsaVoting:announceWinner
function announceWinner() external onlyOwner {
require(block.timestamp >= startVoteTime + duration, "The voting is still active");
require(LINK.balanceOf(address(this)) >= fee, "Not enough LINK - please fill contract with LINK");
uint256 maxVotes = 0;
delete potentialWinners;
for (uint256 i = 0; i < _tokenIds.length; i++) {
uint256 currentVotes = voteCounts[_tokenIds[i]];
if (currentVotes > maxVotes) {
maxVotes = currentVotes;
delete potentialWinners;
potentialWinners.push(_tokenIds[i]);
} else if (currentVotes == maxVotes) {
potentialWinners.push(_tokenIds[i]);
}
}
requestRandomness(keyHash, fee);
}
Add chainlink veritable randomness function
function fulfillRandomness(bytes32 requestId, uint256 randomness) internal override {
uint256 randomResult = randomness % potentialWinners.length;
uint256 winnerTokenId = potentialWinners[randomResult];
list = _martenitsaMarketplace.getListing(winnerTokenId);
_healthToken.distributeHealthToken(list.seller, 1);
emit WinnerAnnounced(winnerTokenId, list.seller);
}
