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

Lack of proper logic in `MartenitsaMarketplace::collectReward` function can lead to collecting rewards unlimited times for same tokens transferred to other wallet.

mirkopezo

Summary

Logic in MartenitsaMarketplace::collectReward function is vulnerable, because user can transfer his tokens to another wallet with MartenitsaMarketplace::makePresent function and also claim reward from that wallet.

Vulnerability Details

User can collect reward, create new wallet and then transfer tokens with MartenitsaMarketplace::makePresent function and also collect reward from that wallet. This can be repeated unlimited times.

@> 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);
}
}

Impact

User can collect unlimited rewards (health tokens).

Proof of Concept

Place the following test into MartenitsaMarketplace.t.sol.

function testCollectRewardUnlimited() public eligibleForReward {
vm.startPrank(bob);
​
marketplace.collectReward();
​
martenitsaToken.setApprovalForAll(address(marketplace), true);
marketplace.makePresent(address(1), 0);
marketplace.makePresent(address(1), 1);
marketplace.makePresent(address(1), 2);
​
vm.startPrank(address(1));
​
marketplace.collectReward();
​
martenitsaToken.setApprovalForAll(address(marketplace), true);
marketplace.makePresent(address(2), 0);
marketplace.makePresent(address(2), 1);
marketplace.makePresent(address(2), 2);
​
vm.startPrank(address(2));
​
marketplace.collectReward();
​
assert(healthToken.balanceOf(bob) == 10 ** 18);
assert(healthToken.balanceOf(address(1)) == 10 ** 18);
assert(healthToken.balanceOf(address(2)) == 10 ** 18);
}

Tools Used

Manual review

Recommendations

One of potential suggestions is to not allow to collect rewards for token ids that were already used to collect reward.

Updates

Lead Judging Commences

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

Multiple addresses

mirkopezo Submitter
over 1 year ago
bube Lead Judge
over 1 year ago
bube Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Multiple addresses

Support

FAQs

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