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

Users Can Claim Rewards With Identical Martenitsas Breaking Protocol Invariant

pacelli

Summary

Users who are not producers and want to get HealthTokens need to buy 3 different Martenitsas in the marketplace in order to be able to claim the token as a reward, the problem is the MartenitsaMarketplace::collectReward function does not check the designs thus allowing users to claim with identical NFTs.

The docs also state rewards can only be claim every 3 NFTs, but the function neither checks the count of NFTs is a multiple of 3.

// no check performed on the design of the NFTs and if the count is multiple of 3
function collectReward() external {
require(!martenitsaToken.isProducer(msg.sender), "You are producer and not eligible for a reward!");
uint256 count = martenitsaToken.getCountMartenitsaTokensOwner(msg.sender);
uint256 amountRewards = (count / requiredMartenitsaTokens) - _collectedRewards[msg.sender];
if (amountRewards > 0) {
_collectedRewards[msg.sender] = amountRewards;
healthToken.distributeHealthToken(msg.sender, amountRewards);
}
}

Vulnerability Details

I've created a proof of concept in the form of a test to show the vulnerability, copy and paste the following test in MartenitsaMarketplace.t.sol:

PoC
function testSecurityReview__UserClaimRewardsWithIdenticalMartenitsas() public {
for (uint256 i; i < 3; i++) {
vm.startPrank(chasy, chasy);
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.approve(address(marketplace), i);
marketplace.listMartenitsaForSale(i, 1 wei);
vm.stopPrank();
}
​
for (uint256 i; i < 3; i++) {
vm.startPrank(bob, bob);
marketplace.buyMartenitsa{value: 1 wei}(i);
vm.stopPrank();
}
​
vm.prank(bob, bob);
marketplace.collectReward();
​
assertEq(healthToken.balanceOf(bob), 1 ether);
}

Impact

Protocol invariant broken, identical NFTs can be used to claim rewards.

Tools Used

Manual review, Foundry

Recommendations

MartenitsaMarketplace::collectRewards needs to be refactor to validate there are not duplicates and the count of NFTs used to claim the rewards is multiple of 3.

The following code snippet shows an example on how the function could be refactored:

-function collectReward() external {
+function collectReward(uint256[] calldata tokenIds) external {
// rest of the function...
​
+ require(tokenIds.length % 3 == 0, "You can only claim every three NFTs");
​
+ bytes32 hashFirst = keccak256(bytes(martenitsaToken.getDesign(tokenIds[0])));
+ for (uint256 i = 1; i < tokenIds.length;) {
+ bytes32 hashNext = keccak256(bytes(martenitsaToken.getDesign(tokenIds[i])));
+ require(hashFirst != hashNext, "You can't claim rewards with identical tokens");
+
+ unchecked {
+ ++i;
+ }
+ }
​
- uint256 count = martenitsaToken.getCountMartenitsaTokensOwner(msg.sender);
- uint256 amountRewards = (count / requiredMartenitsaTokens) - _collectedRewards[msg.sender];
+ uint256 amountRewards = (tokenIds.length / requiredMartenitsaTokens) - _collectedRewards[msg.sender];
// rest of the function
}

The function receives an array of type uint256 as a parameter with the list of token ids the user wants to use to claim the reward. The length of the array is validated to be multiple of 3 so users can only claim using 3, 6, 9 tokens and so on.

Then, to check if there are duplicated NFTs we obtain the hash of the design of the token at the zero index to use it for comparison, later a loop is executed on the rest of the tokens in the array to hash their design and compare each one against the hash of NFT at the zero index. If a duplicate is found the execution reverts.

Updates

Lead Judging Commences

bube Lead Judge
over 1 year ago
bube Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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