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

The array `MartenitsaEvent::participants` is not emptied after stopping the event and it can become too expensive to finish it later on.

robertodf99

Summary

When the event finishes, the owner of the contract has to call the function MartenitsaEvent::stopEvent to remove the producer role from each participant. This is done by iterating in a for loop through each particpant and setting the value of the mapping MartenitsaEvent::isProducer to false. Since the array MartenitsaEvent::participants used in the for loop is not emptied after the event finishes it can become too expensive in the future to call this function.

Vulnerability Details

Every time a participant joins the event, its address is added to the array MartenitsaEvent::participants. When the deadline is met and the owner calls MartenitsaEvent::stopEvent to set back to the previous state the roles of the users, it forgets to empty this array. Since the rolling back to the previous role is performed using a for loop, as the array becomes bigger, calling this function may become economically inviable. This is worsened by the poor gas optimization techniques being used such as not caching the array length in memory.

Impact

As a consequence of not emptying the array, the gas consumed in a function call to MartenitsaEvent::stopEvent will become more and more expensive. To get an estimate of the magnitude, the test below is executed for different number of particpants, determined by the iterations in the for loops.

Code

Place this into MartenitsaEvent.t.sol.

function test_stopEventDOS() public {
martenitsaEvent.startEvent(1 days);
vm.startPrank(chasy);
uint256 k;
​
for (uint256 i = 1; i < 100001; i++) {
for (uint256 j; j < 3; j++) {
address receiver = address(uint160(i));
martenitsaToken.createMartenitsa("bracelet");
martenitsaToken.approve(address(marketplace), k);
marketplace.makePresent(receiver, k);
k = k + 1;
}
}
vm.stopPrank();
for (uint256 i = 1; i < 100001; i++) {
vm.startPrank(address(uint160(i)));
marketplace.collectReward();
healthToken.approve(address(martenitsaEvent), 10 ** 18);
martenitsaEvent.joinEvent();
vm.stopPrank();
}
vm.warp(block.timestamp + 1 days + 1);
uint256 gasBefore = gasleft();
martenitsaEvent.stopEvent();
uint256 gasAfter = gasleft();
console.log("gas used: %d", gasBefore - gasAfter);
}

The gas consumed in the test for the different iterations are:

  • 1,000 iterations: 780,180 gas

  • 10,000 iterations: 7,791,180 gas

  • 100,000 iterations: 77,901,180 gas

It can be observed that the gas increases at a rate of approximately 779 gas per particpant joining the event. Considering that Ethereum is the chosen network to deploy the protocol, it can be something very problematic during periods of high network congestion as the gas price will be considerably high.

Tools Used

Foundry and manual review.

Recommendations

Implement an optimized code that deletes the participants array and caches the array length in memory to avoid reading from storage in every iteration.

Code
function stopEvent() external onlyOwner {
require(block.timestamp >= eventEndTime, "Event is not ended");
uint256 partcipantsLength = participants.length;
for (uint256 i = 0; i < partcipantsLength; i++) {
isProducer[participants[i]] = false;
delete participants[i];
}
}
Updates

Lead Judging Commences

bube Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.