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

Wrong calculation in `collectReward` function allows to get more `health tokens` to `Producer`

naman1729

Summary

  • collectReward function is not working as expected. Producer can get more health tokens than expected.

Vulnerability Details

  • Producer can get more health tokens than expected. For example, if a producer has 9 martenitsa tokens then he should get 3 health tokens but he is able to get 4 health tokens because of the wrong calculation in the collectReward function of MartenitsaMarketplace.sol contract. he can get more 4 health tokens if he call the collectReward function at 3 martenitsa tokens then he will get 1 health tokens and when he have 6 martenitsa tokens then he call the collectReward function then he have 2 health tokens but _collectedRewards mapping is not updated accuretly. So, he can get more health tokens than expected on 9 martenitsa tokens.

  • then he call the collectReward function when he have 9 martenitsa tokens then he will get 4 health tokens because of _collectedRewards shows only 1 health tokens claimed by the producer this will allow the producer to get 4 health tokens on 9 martenitsa tokens.

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

POC

  • Put this test in MartenitsaMarketplace.t.sol.

function testCollectReward() public {
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
marketplace.listMartenitsaForSale(0, 1 wei);
marketplace.listMartenitsaForSale(1, 1 wei);
marketplace.listMartenitsaForSale(2, 1 wei);
martenitsaToken.approve(address(marketplace), 0);
martenitsaToken.approve(address(marketplace), 1);
martenitsaToken.approve(address(marketplace), 2);
marketplace.makePresent(bob, 0);
marketplace.makePresent(bob, 1);
marketplace.makePresent(bob, 2);
vm.stopPrank();
​
vm.startPrank(bob);
marketplace.collectReward();
vm.stopPrank();
assert(healthToken.balanceOf(bob) == 1 * 1e18);
​
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
marketplace.listMartenitsaForSale(3, 1 wei);
marketplace.listMartenitsaForSale(4, 1 wei);
marketplace.listMartenitsaForSale(5, 1 wei);
martenitsaToken.approve(address(marketplace), 3);
martenitsaToken.approve(address(marketplace), 4);
martenitsaToken.approve(address(marketplace), 5);
marketplace.makePresent(bob, 3);
marketplace.makePresent(bob, 4);
marketplace.makePresent(bob, 5);
vm.stopPrank();
​
vm.startPrank(bob);
marketplace.collectReward();
vm.stopPrank();
assert(healthToken.balanceOf(bob) == 2 * 1e18);
​
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
marketplace.listMartenitsaForSale(6, 1 wei);
marketplace.listMartenitsaForSale(7, 1 wei);
marketplace.listMartenitsaForSale(8, 1 wei);
martenitsaToken.approve(address(marketplace), 6);
martenitsaToken.approve(address(marketplace), 7);
martenitsaToken.approve(address(marketplace), 8);
marketplace.makePresent(bob, 6);
marketplace.makePresent(bob, 7);
marketplace.makePresent(bob, 8);
vm.stopPrank();
​
vm.startPrank(bob);
marketplace.collectReward();
vm.stopPrank();
console.log(healthToken.balanceOf(bob));
assert(healthToken.balanceOf(bob) == 4 * 1e18);
// healthToken.balanceOf(bob) = 3 * 1e18 for 9 martenitsa but above test shows that it is able to get 4 * 1e18
// because of the wrong calculation in collectReward function
}

  • Run this test by this command.

forge test --mt testCollectReward -vvvv

Impact

  • collectReward function is not working as expected.

  • Producer can get more health tokens than expected.

Tools Used

  • Manual review

Recommendations

  • Make these changes in the collectReward function of MartenitsaMarketplace.sol contract.

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
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.