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

Potential DOS Attack in `martenitsaToken::getAllProducers` View Function

0x77

Description

The array storage variable martenitsaToken::producers could lead to a DOS (Denial of Service) attack if it becomes very large. In Solidity, attempting to operate on or return large arrays can exceed the block gas limit, causing transaction failures.

Impact

If the function martenitsaToken::getAllProducers returns a very large array (e.g., containing 10,000 elements), it could exceed the gas limit of a typical transaction, making it impossible to successfully call the function.

Proof of Concepts

  1. A large number of producers are added via martenitsaToken::setProducers. Although under normal circumstances this function is only callable by the owner, which should theoretically prevent the addition of excessively many producers, the martenitsaEvent::joinEvent function in the martenitsaEvent contract includes a _addProducer step. This allows the addresses participating in the event to be added as producers, potentially leading to a DOS attack.

  2. After adding a large number of producers, calling martenitsaToken::getAllProducers will fail because the gas exceeds the block's maximum limit.

Proof of code

Add the following code to "MartenitsaToken.t.sol" for simulation:

function testSetProducers() public {
address[] memory producersList = new address[](100000);
​
for (uint256 i = 0; i < 100000; i++) {
producersList[i] = vm.addr(i + 1);
}
martenitsaToken.setProducers(producersList);
}
​
function testGetAllProduers() public {
testSetProducers();
​
vm.startPrank(bob);
try martenitsaToken.getAllProducers() returns (
address[] memory producers
) {
fail("should hava get gas error");
} catch {
emit log("ouf of gas");
}
vm.stopPrank();
}

Recommended mitigation

To mitigate the risk of a denial of service (DOS) attack stemming from excessive gas consumption, it is recommended to modify the storage structure from an array to a mapping. This change prevents the need to return the entire array of producers, which is inefficient and potentially problematic for large data sets. Implement a mapping that allows accessing individual producers via an index, and introduce a counter to track the total number of producers. This approach enhances gas efficiency and scalability by facilitating access to specific producers without the overhead associated with large array operations.

- address[] public producers;
+ uint256 public producersCount;
+ mapping(uint256 => address) public producers;
​
function setProducers(address[] memory _producersList) public onlyOwner{
for (uint256 i = 0; i < _producersList.length; i++) {
isProducer[_producersList[i]] = true;
- producers.push(_producersList[i]);
+ producers[producersCount] = _producersList[i];
+ producersCount++;
}
}
​
- function getAllProducers() external view returns (address[] memory) {
- return producers;
- }
+ function getProducers(uint256 index) external view returns (address memory) {
+ return producers[index];
+ }
​
function _addProducer(address _producer) internal {
isProducer[_producer] = true;
- producers.push(_producer);
+ producers[producersCount] = _producer;
+ producersCount++;
}

This change enhances performance and avoids potential out-of-gas errors by limiting the data returned in a single call, thus improving the contract's resilience against DOS attacks.

Updates

Lead Judging Commences

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.