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

Missing access control in `MartenitsaToken::updateCountMartenitsaTokensOwner`

Summary

The function MartenitsaToken::updateCountMartenitsaTokensOwner allows any user to adjust their martenitsa token balances, whether increasing or decreasing them.

Vulnerability Details

The function MartenitsaToken::updateCountMartenitsaTokensOwner is used to update the amount of martenitsas owned by users with non-producer role. These users are eligible to claim health tokens once they possess a minimum required amount of martenitsas by calling MartenitsaMarketplace::collectReward, and this function relies on MartenitsaToken::getCountMartenitsaTokensOwner which can be manipulated through MartenitsaToken::updateCountMartenitsaTokensOwner. Therefore, only the marketplace should have permission to call this last function, however, the current contract code overlooks this crucial check.

Impact

Any arbitrary user can make use of this function to increase or decrease balances as they see fit.

One attack vector could be the increase in the own attacker's balance to later claim health tokens without actually owning the minimum amount of martenitsas required. The steps to perform such attack can be delineated as simple as:

  • attacker calls three consecutive times to the function MartenitsaToken::updateCountMartenitsaTokensOwner passing his/her address and add as the function call parameters.

  • Immediately after calls MartenitsaMarketplace::collectReward. As a result now the attacker owns 1 health token and has acquired 0 martenitsas.

See PoC below:

Code

Place this in MartenitsaToken.t.sol.

function test_attackerCanIncreaseBalance() public {
address attacker = makeAddr("attacker");
vm.startPrank(attacker);
for (uint256 i; i < 3; i++) {
martenitsaToken.updateCountMartenitsaTokensOwner(attacker, "add");
}
marketplace.collectReward();
assertEq(healthToken.balanceOf(attacker), 10 ** 18);
}

A second attack vector would imply the attacker reducing other users' martenitsas token balance. For this attack the attacker would proceed as follows:

  • victim mints the minimum amount required of martenitsas to claim the health token.

  • attacker calls three times MartenitsaToken::updateCountMartenitsaTokensOwner passing the victim's address and sub as the function call parameters.

  • victim calls MartenitsaMarketplace::collectReward but since his/her balance is 0 as a result of the attacker's calls, he/she does not have the right to claim for the corresponding health token.

See PoC below:

Code

Place this in MartenitsaToken.t.sol.

function test_attackerDecreaseVictimBalance() public eligibleForReward {
address attacker = makeAddr("attacker");
address victim = bob;
vm.startPrank(attacker);
for (uint256 i; i < 3; i++) {
martenitsaToken.updateCountMartenitsaTokensOwner(victim, "sub");
}
vm.stopPrank();
vm.prank(victim);
marketplace.collectReward();
assertEq(healthToken.balanceOf(victim), 0);
}

Tools Used

Foundry, manual review.

Recommendations

Restrict the access to the function to only the marketplace address. For example, a function can be added to set the address of the marketplace and then check at the beginning of MartenitsaToken::updateCountMartenitsaTokensOwner if the caller matches the marketpace address, if not, revert. It can be easily implemented using a require:

import {MartenitsaMarketplace} from "./MartenitsaMarketplace.sol";
...
function setMarketAddress(address martenitsaMarketplace) public onlyOwner {
_martenitsaMarketplace = MartenitsaMarketplace(martenitsaMarketplace);
}
...
function updateCountMartenitsaTokensOwner(
address owner,
string memory operation
) external {
+ require(
+ msg.sender == address(_martenitsaMarketplace),
+ "Caller should be marketplace!"
+ );
...
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.