Summary
The MartenitsaMarketplace::getListing reverts in case the token is not for sale, therefore the martenitsa token which was on sale earlier when bought will get unlisted and getListing function will revert.
In MartenitsaVoting contract when the voting period is over, the winner is announced by owner, but if the token is unlisted from sale on the marketplace when a buyer buys it, the getListing function will revert and as a result of which the winner can never be announced and will not receive their HealthToken reward.
Vulnerability Details
The vulnerability is present in the MartenitsaVoting contract where it uses MartenitsaMarketplace::getListing function to get the address of producer associated with the winning martenitsa token id, but getListing function reverting upon the token getting unlisted when one buys it will lead to winner not getting their HealthToken reward and there will no winner announced as announceWinner function will revert due to revert from MartenitsaMarketplace.
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
PoC
Add the test in the file: test/MartenitsaVoting.t.sol
Run the test:
forge test --mt test_WinnerTokenBeingBought_LeadsTo_RevertInAnnounceWinner
function test_WinnerTokenBeingBought_LeadsTo_RevertInAnnounceWinner() public {
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.approve(address(marketplace), 0);
marketplace.listMartenitsaForSale(0, 1 wei);
vm.stopPrank();
voting.startVoting();
vm.prank(bob);
voting.voteForMartenitsa(0);
vm.warp(block.timestamp + voting.duration());
vm.prank(bob);
marketplace.buyMartenitsa{value: 1 wei}(0);
vm.expectRevert("Token is not listed for sale");
voting.announceWinner();
}
Tools Used
Manual Review, Foundry Unit Test
Recommendations
When a token receives their first vote, store the seller address in the voting contract.
@@ -20,6 +20,8 @@ contract MartenitsaVoting is Ownable {
//mapping user address -> if this address is already voted
mapping(address => bool) public hasVoted;
+ mapping(uint256 tokenId => address candidate) public tokenIdToCandidate;
+
event WinnerAnnounced(uint256 indexed winnerTokenId, address indexed winner);
event Voting(uint256 indexed startTime, uint256 indexed duration);
@@ -46,6 +48,10 @@ contract MartenitsaVoting is Ownable {
list = _martenitsaMarketplace.getListing(tokenId);
require(list.forSale, "You are unable to vote for this martenitsa");
+ if (tokenIdToCandidate[tokenId] == address(0)) {
+ tokenIdToCandidate[tokenId] = list.seller;
+ }
+
hasVoted[msg.sender] = true;
voteCounts[tokenId] += 1;
_tokenIds.push(tokenId);
@@ -67,8 +73,7 @@ contract MartenitsaVoting is Ownable {
}
}
- list = _martenitsaMarketplace.getListing(winnerTokenId);
- _healthToken.distributeHealthToken(list.seller, 1);
+ _healthToken.distributeHealthToken(tokenIdToCandidate[winnerTokenId], 1);
emit WinnerAnnounced(winnerTokenId, list.seller);
}