Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Denial of Service (DoS) - Gas Limit Issue in distributeProtocolAssetReward Function of MarketMakingEngineConfiguration Contract

sonamakash54

Summary

The distributeProtocolAssetReward function in the MarketMakingEngineConfiguration contract has a potential gas issue due to the large number of fee recipients being processed. The function fails with an "OutOfGas" error when the number of fee recipients increases significantly.

  • Severity: High

  • Impact: High

  • Likelihood: Medium

Vulnerability Details

The issue arises from the for-loop in the distributeProtocolAssetReward function that iterates over all the fee recipients. When there is a large number of fee recipients (e.g., 12,000 in this case), the gas cost to process the loop and transfer the rewards becomes too high, causing the transaction to fail due to the out-of-gas error. This results in the function not being able to execute successfully.

  • Root Cause: High gas cost for looping over a large number of fee recipients.

  • Impact: The function may fail to distribute protocol rewards properly when the number of recipients is large, making the contract's functionality unreliable in cases with many fee recipients.

Impact

  • Denial of Service (DoS): The inability to distribute rewards to a large number of recipients may lead to service denial, as the contract may become stuck or fail to execute on such calls.

  • Operational Risk: This can affect users relying on the reward distribution mechanism for governance or other incentives, potentially leading to economic losses or contract failure.

POC (with Steps to Reproduce, and also add test and test result)

  1. Steps to Reproduce:

    • Deploy the MarketMakingEngineConfiguration contract.

    • Add a large number of fee recipients (12,000 in this case).

    • Attempt to distribute protocol asset rewards to these recipients using the distributeProtocolAssetReward function.

  2. Test Code:

    function test_DoS_in_distributeProtocolAssetReward() public {
    vm.expectRevert("gas out of gas"); // Expect gas-related revert
    // Try calling the distribute function with increased computations
    uint256 rewardAmount = 1e27; // Example reward to distribute
    MarketMakingEngineConfiguration.distributeProtocolAssetReward(marketConfig, address(asset), rewardAmount);
    }

  3. Test Result:

    • The test case failed with an "OutOfGas" error when executing the setUp() function, indicating that the gas usage exceeded the limit due to the large number of fee recipients.

    [FAIL: EvmError: OutOfGas] setUp() (gas: 0)

Tools Used

Manual Review
Foundry

Recommendations

  1. Batch Processing: Instead of processing all fee recipients in a single transaction, implement batch processing or pagination for distributing rewards. This will reduce the gas cost per transaction and allow the contract to process rewards in multiple steps if necessary.

  2. Gas Optimization: Consider optimizing the gas consumption in the loop, such as by caching values that do not change across iterations or optimizing the reward calculation logic.

  3. Gas Limit Monitoring: Implement proper gas limit checks and reverts to handle cases where the gas consumption might exceed the block's gas limit, ensuring that transactions fail gracefully and don't hang.

  4. Testing: Perform additional tests with varying numbers of fee recipients to evaluate the gas impact and ensure that the function can handle larger configurations without running into gas limit issues.

    Recommendations

    Batch Processing: Instead

// Optimized distributeProtocolAssetReward with batch processing
function distributeProtocolAssetReward(Data storage self, address asset, uint256 amount) internal {
uint256 feeRecipientsLength = self.protocolFeeRecipients.length();
UD60x18 totalFeeRecipientsSharesX18 = ud60x18(self.totalFeeRecipientsShares);
UD60x18 amountX18 = ud60x18(amount);
​
uint256 totalDistributed = 0;
uint256 batchSize = 100; // Processing 100 recipients per batch
​
for (uint256 i; i < feeRecipientsLength; i++) {
if (i % batchSize == 0 && i > 0) {
// Emit event or handle if necessary for current batch
emit LogGasLimitReached(i);
break; // Break to avoid gas overage and process the next batch
}
​
(address feeRecipient, uint256 shares) = self.protocolFeeRecipients.at(i);
​
uint256 feeRecipientReward = amountX18.mul(ud60x18(shares)).div(totalFeeRecipientsSharesX18).intoUint256();
totalDistributed += feeRecipientReward;
​
// Ensure that the last recipient gets the remaining amount
if (i == feeRecipientsLength - 1) {
feeRecipientReward += amountX18.sub(ud60x18(totalDistributed)).intoUint256();
}
​
IERC20(asset).safeTransfer(feeRecipient, feeRecipientReward);
}
}
​
// Log event for batch processing
event LogGasLimitReached(uint256 recipientsProcessed);
}

  • Here’s an enhanced test suite that demonstrates the vulnerability and provides clear expectations for judges:


    forge test test/foundry/MarketMakingEngineConfiguration.t.sol

// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
​
import "forge-std/Test.sol";
import "@openzeppelin/token/ERC20/IERC20.sol";
import "@openzeppelin/token/ERC20/ERC20.sol";
import "@openzeppelin/utils/structs/EnumerableMap.sol";
import "../../src/market-making/leaves/MarketMakingEngineConfiguration.sol"; // Import the contract to test
​
contract MarketMakingEngineDoSTest is Test {
using EnumerableMap for EnumerableMap.AddressToUintMap;
​
MarketMakingEngineConfiguration.Data marketConfig;
MockERC20 asset;
address[] feeRecipients;
​
uint256 constant TOTAL_SHARES = 1e18; // 100% in 18 decimals
uint256 constant NUM_RECIPIENTS = 12000; // Increased to trigger gas issues
​
function setUp() public {
// Deploy a mock ERC20 token
asset = new MockERC20("Mock Token", "MOCK");
​
// Initialize the MarketMakingEngineConfiguration
marketConfig.totalFeeRecipientsShares = uint128(TOTAL_SHARES);
​
// Add a larger number of fee recipients to increase computational load
for (uint256 i = 0; i < NUM_RECIPIENTS; i++) {
address recipient = address(uint160(i + 1)); // Generate dummy addresses
marketConfig.protocolFeeRecipients.set(recipient, TOTAL_SHARES / NUM_RECIPIENTS);
feeRecipients.push(recipient);
}
​
// Mint and approve asset for distribution
asset.mint(address(this), 1e30); // Mint a large amount to prevent reverts during transfer
asset.approve(address(this), type(uint256).max);
}
​
function test_DoS_in_distributeProtocolAssetReward() public {
// Expect the function to revert due to gas issues
vm.expectRevert("gas out of gas"); // Expect gas-related revert
// vm.expectRevert(); both works and the out come is same
// [FAIL: EvmError: OutOfGas]
​
// Try calling the distribute function with increased computations
uint256 rewardAmount = 1e27; // Example reward to distribute
MarketMakingEngineConfiguration.distributeProtocolAssetReward(marketConfig, address(asset), rewardAmount);
}
​
// Additional check for gas usage (optional)
function testGasUsage_in_distributeProtocolAssetReward() public {
uint256 initialGas = gasleft(); // Capture the gas before function call
​
// Try to distribute protocol asset reward with more fee recipients
MarketMakingEngineConfiguration.distributeProtocolAssetReward(marketConfig, address(asset), 1e27);
​
uint256 usedGas = initialGas - gasleft(); // Calculate used gas
emit log_named_uint("Gas used in distributeProtocolAssetReward", usedGas);
// Log the gas usage to better understand whether the gas issue occurs in your scenario
}
}
​
// Mock ERC20 token for testing
contract MockERC20 is ERC20 {
constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
​
function mint(address to, uint256 amount) public {
_mint(to, amount);
}
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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