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

`MartenitsaEvent::stopEvent` loops through an unbounded array, risk of eDoS/DoS

paprikrumplikas

Summary

MartenitsaEvent::stopEvent loops through the unbounded array participants, making this function vulnerable to economic Denial of Service (eDoS) or even regular DoS attacks.

Vulnerability Details

MartenitsaEvent::stopEvent is designed to reset the producer status (tracked by mapping isProducer) of all participants at the end of an event. To perform this reset, the function iterates over the participants array that, however, is an unbounded array: there is no limit on the number of participants that can join an event. Consequently, this function is vulnerable to economic Denial of Service (eDoS) or even regular DoS attacks.

(eDoS: Executing the stopEvent function would not hit the gas limit, thus avoiding a complete Denial of Service (DoS), but it would consumes an inordinate amount of gas that would makes the transaction economically unfeasible for the protocol owner to perform.)

The following test simulates a scenario where 40,000 participants join an event, which results in the stopEvent function consuming an amount of gas that would exceed typical block gas limits today (30 million):

Proof of Code
function testDOSdueToUnboundedLoop() public {
uint256 numParticipants = 40000;
address[] memory participants2 = new address[](numParticipants);
​
// setting trx gas price to 1
vm.txGasPrice(1);
​
// event starts
martenitsaEvent.startEvent(1 days);
​
for (uint256 i = 0; i < numParticipants; i++) {
address currentParticipant;
participants2[i] = address(uint160(i + 1)); //skip zero address
currentParticipant = participants2[i];
​
// participants somehow acquire 1 HT each
// here we deal them one, but they can get one from someone, buy on DEX, get from rewards, etc.
deal(address(healthToken), currentParticipant, 1 ether);
​
// participants enter the event
vm.startPrank(currentParticipant);
healthToken.approve(address(martenitsaEvent), 1 ether);
martenitsaEvent.joinEvent();
vm.stopPrank();
}
​
// event ends
vm.warp(block.timestamp + 1 days + 1 seconds);
uint256 gasStart = gasleft();
martenitsaEvent.stopEvent();
uint256 gasEnd = gasleft();
​
// gas limit is currently 30 million, which is now exceeded
console.log("Gas: ", gasStart - gasEnd);
}

As seen, hitting the gas limit would require about 40,000 participants. While this number might seem big, consider that

  • it is not inconceivable, and not without precedent, that a protocol gets so popular that even more people want to join its events;

  • participants are economacially incentivized to perform a DoS to retain their producer status forever;

  • performing an eDoS requires a lot less participants, but effectively gives the same results.

Impact

Issues arise even without a DoS or eDoS:

  • Increased Transaction Costs: Even without malicious intent, regular usage could lead to high gas costs. The more legitimate users join events, the more the cost of calling stopEvent will be.

  • Contract Clogging: With the participants array not being cleared (another bug), repeated events can lead to unchecked growth in its size, further exacerbating the issue.

A DoS or eDoS would result in:

  • Irreversible Temporary Statuses: Participants from the event will indefinitely maintain their producer status.

  • Economic Manipulation: Retaining their producer status allows participants to indefinitely create, list, and sell Martenitsa NFTs. This unlimited production and sale capability can lead to market manipulation and unfair economic advantages, as these users exploit their persistent producer privileges without temporal restrictions.

Tools Used

Manual review, Foundry.

Recommendations

Consider implementing one or more of the following changes:

  • Limit the number of participants:

+ uint256 public constant MAX_PARTICIPANTS = 1000;
​
...
​
function joinEvent() external {
+ require(participants.length < MAX_PARTICIPANTS, "Participant limit reached");
...
}

You can consider doing this in a more sophisticated manner: e.g. declaring the max number of participants as a non-constant variable, and add a function with which you could adjust its value within reasonable, safe limits. Additionally, if your events get super popular, you can just run more events after each other.

  • Clear the participants array in MartenitsaEvent::stopEvent:

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

Lead Judging Commences

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

Unbounded arrays

paprikrumplikas 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.