Beginner FriendlyFoundryGameFi
100 EXP
View results
Submission Details
Severity: high
Valid

`MartenitsaMarketplace::_collectedRewards` is not correctly updated inside `collectReward` function allowing for unlimited HealthToken mints just by holding 6 different MartenitsaTokens.

Summary

The function collectReward is intended to allow users to mint 1 HealthToken for every 3 different
MartenitsaTokens they held.

Inside it's implementation, the function fails to aggregate the number of rewards already collected by
the user. Instead it sets _collectedRewards[msg.sender] to the latest amount of rewards claimed by the user.

Vulnerability Details

By buying 6 MartenitsaTokens an attacker should be able to mint an unlimited amount of HealthTokens.

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;
// @-audit the line above should use += instead.
healthToken.distributeHealthToken(msg.sender, amountRewards);
}
}

Proof of concept

  1. Attacker buys 3 distinct tokens

  2. Attacker calls collectReward to receive one HealthToken

  3. Attacker buys 3 extra distinct tokens

  4. Attacker calls collectReward to receive one additional HealthToken

  5. Repeat 4 an unlimited number of times.

PoC

Place the following code in MartenitsaMarketplace.t.sol.

function testUnlimitedHealthTokenMints6DistinctTokens() public {
address attacker = makeAddr("attacker");
// Get the first 3 tokens
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet1");
martenitsaToken.createMartenitsa("bracelet2");
martenitsaToken.createMartenitsa("bracelet3");
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();
assert(martenitsaToken.balanceOf(address(attacker)) == 3);
vm.prank(attacker);
marketplace.collectReward();
assert(healthToken.balanceOf(address(attacker)) == 10 ** 18);
// Get the sedond set of 3 tokens
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet4");
martenitsaToken.createMartenitsa("bracelet5");
martenitsaToken.createMartenitsa("bracelet6");
martenitsaToken.approve(address(marketplace), 3);
martenitsaToken.approve(address(marketplace), 4);
martenitsaToken.approve(address(marketplace), 5);
marketplace.makePresent(attacker, 3);
marketplace.makePresent(attacker, 4);
marketplace.makePresent(attacker, 5);
vm.stopPrank();
assert(martenitsaToken.balanceOf(address(attacker)) == 6);
vm.prank(attacker);
marketplace.collectReward();
assert(healthToken.balanceOf(address(attacker)) == 2 * 10 ** 18);
// Repeat the last call to mint an unlimited amount of health tokens
vm.prank(attacker);
marketplace.collectReward();
assert(healthToken.balanceOf(address(attacker)) == 3 * 10 ** 18);
}

Impact

It's possible to mint an unlimited amount of HealthTokens by holding 6 distinct MartenitsaTokens.

Tools Used

Manual review

Recommended Mitigation

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.