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

Broken Access control in `MartenitsaToken:updateCountMartenitsaTokensOwner` function

blacksquirrel

Summary

Description: MartenitsaToken:updateCountMartenitsaTokensOwner function is called in MartenitsaMarketplace contract in functions buyMartenitsa and makePresent to update value of mapping countMartenitsaTokensOwner , which stores amount of Martenitsa tokens user has .

function updateCountMartenitsaTokensOwner(address owner, string memory operation) external {
if (keccak256(abi.encodePacked(operation)) == keccak256(abi.encodePacked("add"))) {
- countMartenitsaTokensOwner[owner] += 1; // adds one Martinitsa token to user
} else if (keccak256(abi.encodePacked(operation)) == keccak256(abi.encodePacked("sub"))) {
countMartenitsaTokensOwner[owner] -= 1;
} else {
revert("Wrong operation");
}
}

But its visibility is set to external and anybody can call it and exploit this function.

Vulnerability Details

Proof of Concept:

  1. malicious user can pretend having Martenitsa tokens calling MartenitsaToken:updateCountMartenitsaTokensOwner function unlimited times,
    because it updates mapping countMartenitsaTokensOwner and has no access controls.

  2. then mint health tokens using MartenitsaMarketplace:collectReward function because it calculates health tokens checking mapping but not abalance of user.

Proof of Code:

Code for Testing

Add the following code to the MartenitsaToken.t.sol file.

function testMintHealthTokenForFree() public {
uint256 number = 300;
​
for (number = 0; number < 300; number++) {
martenitsaToken.updateCountMartenitsaTokensOwner(bob, "add");
}
console.log("Bob has 300 MartinisaTokens :", martenitsaToken.getCountMartenitsaTokensOwner(bob));
​
vm.prank(bob);
marketplace.collectReward();
​
console.log("Bob has 100 Healthtokens :", healthToken.balanceOf(bob) / 1e18);
​
assertEq(martenitsaToken.getCountMartenitsaTokensOwner(bob), 300);
assertEq(healthToken.balanceOf(bob), 100e18);
}

Impact

A user does not need to buy Martinitsa tokens or be granted with them to mint health tokens because he can
exploit MartenitsaToken:updateCountMartenitsaTokensOwner function in his favor.

Tools Used

Manual review.

Recommendations

Additional access control code must be implemented in MartenitsaToken contract , as it was implemented
in HealthToken contract. Only MartenitsaMarketplace contract can call updateCountMartenitsaTokensOwner function.

Code for Adding
​
+ MartenitsaMarketplace private _martenitsaMarketplace;
​
+ function setMarketAddress(address martenitsaMarketplace) public onlyOwner {
+ _martenitsaMarketplace = MartenitsaMarketplace(martenitsaMarketplace);
}
​
function updateCountMartenitsaTokensOwner(address owner, string memory operation) external {
+ require(msg.sender == address(_martenitsaMarketplace), "Ups !!!");
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");
}
}
​
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.