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

Looping through `MartenitsaEvent::participants` storage array in `MartenitsaEvent::stopEvent` function can lead to denial of service to stop the event.

mirkopezo

Summary

Participants are joining event, MartenitsaEvent::participants arrays grows in size. Participants array can grow so big that it will not be possible to stop event by calling MartenitsaEvent::stopEvent function.

Vulnerability Details

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

Impact

If MartenitsaEvent::participants grows big, it will never be possible to stop the event. It means that protocol is basically unusable after that, it will not be able to correctly start another event because MartenitsaEvent::stopEvent function has important logic to clear producers mapping.

Proof of Concept

Place the following test into MartenitsaEvent.t.sol. Import console from forge-std/Test.sol.

As MartenitsaEvent::participants array grows, it will cost substantially more gas to stop the event.

function testStopEventDoS() public {
vm.txGasPrice(1);
uint256 healthTokenRequirement = martenitsaEvent.healthTokenRequirement();
​
martenitsaEvent.startEvent(1 days);
​
for (uint256 i = 1; i <= 100; i++) {
address addr = address(uint160(i));
deal(address(healthToken), addr, healthTokenRequirement);
vm.startPrank(address(uint160(i)));
healthToken.approve(address(martenitsaEvent), healthTokenRequirement);
martenitsaEvent.joinEvent();
}
​
vm.warp(block.timestamp + 1 days + 1);
vm.stopPrank();
uint256 gasBefore = gasleft();
martenitsaEvent.stopEvent();
uint256 gasAfter = gasleft();
uint256 gasTotal = gasBefore - gasAfter;
console.log("Gas cost to stop first event with 100 participants: %s", gasTotal);
​
martenitsaEvent.startEvent(1 days);
​
for (uint256 i = 101; i <= 200; i++) {
address addr = address(uint160(i));
deal(address(healthToken), addr, healthTokenRequirement);
vm.startPrank(address(uint160(i)));
healthToken.approve(address(martenitsaEvent), healthTokenRequirement);
martenitsaEvent.joinEvent();
}
​
vm.warp(block.timestamp + 1 days + 1);
vm.stopPrank();
uint256 gasBefore2 = gasleft();
martenitsaEvent.stopEvent();
uint256 gasAfter2 = gasleft();
uint256 gasTotal2 = gasBefore2 - gasAfter2;
console.log("Gas cost to stop second event with 100 participants: %s", gasTotal2);
​
assert(gasTotal2 > gasTotal);
}

Test output:

Gas cost to stop first event with 100 participants: 79080
Gas cost to stop second event with 100 participants: 156980

Tools Used

Manual review

Recommendations

Change logic of MartenitsaEvent::stopEvent function so there is no looping through dynamic storage array.

One potential suggestion is to have mapping(uint256 eventId => mapping(address => bool)) isProducer so there is no need to clear whole mapping for each event.

Updates

Lead Judging Commences

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

Unbounded arrays

mirkopezo Submitter
over 1 year ago
bube Lead Judge
over 1 year ago
bube Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Unbounded arrays

Support

FAQs

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