Lines of code

https://github.com/code-423n4/2024-06-panoptic/blob/main/contracts/base/Multicall.sol#L12-L36

Vulnerability details

Impact

An attacker could exploit the delegatecall-loop vulnerability by crafting malicious data that causes unexpected behavior when the delegatecall is executed inside a loop in a payable function. This could potentially result in unauthorized access to sensitive data, manipulation of contract state, or loss of funds. By repeatedly calling delegatecall within the loop, an attacker could potentially bypass security checks or manipulate the flow of execution in a way that benefits them. This vulnerability poses a high risk as it could lead to severe consequences if exploited by a malicious actor.

Proof of Concept

Position : https://github.com/code-423n4/2024-06-panoptic/blob/main/contracts/base/Multicall.sol#L12-L36

Code :

function multicall(bytes[] calldata data) public payable returns (bytes[] memory results) {
    results = new bytes[](data.length);
    for (uint256 i = 0; i &lt; data.length; ) {
        (bool success, bytes memory result) = address(this).delegatecall(data[i]);

        if (!success) {
            // Bubble up the revert reason
            // The bytes type is ABI encoded as a length-prefixed byte array
            // So we simply need to add 32 to the pointer to get the start of the data
            // And then revert with the size loaded from the first 32 bytes
            // Other solutions will do work to differentiate the revert reasons and provide paranthetical information
            // However, we have chosen to simply replicate the the normal behavior of the call
            // NOTE: memory-safe because it reads from memory already allocated by solidity (the bytes memory result)
            assembly ("memory-safe") {
                revert(add(result, 32), mload(result))
            }
        }

        results[i] = result;

        unchecked {
            ++i;
        }
    }
}

Tools Used

SET IN STONE : https://lab.setinstone.io

Recommended Mitigation Steps

The vulnerability in the provided code snippet is the presence of a delegatecall inside a loop in a payable function. This can lead to potential reentrancy attacks, where an attacker can repeatedly call the multicall function and potentially drain the contract's Ether balance.

To rectify this vulnerability, you should ensure that the function being called by delegatecall is not payable and does not use msg.value. Here's a recommended approach:

Create a modifier , for instance onlyOwner, that restricts access to certain functions to only authorized users ( ie : the contract owner).

address public owner;

constructor() { owner = msg.sender; // Setting the contract deployer as the owner }

modifier onlyOwner() { require(msg.sender == owner, "Caller is not the owner"); _; }


2. Make the multicall function non-payable and restrict its access to only the contract owner using the onlyOwner modifier.

function multicall(bytes[] calldata data) public onlyOwner returns (bytes[] memory results) { // ... (existing code) }



3. Ensure that the functions being called by delegatecall are not payable and do not use msg.value. If they need to handle Ether transfers, implement separate functions that can be called directly by the contract owner.

By following these steps, you can mitigate the risk of reentrancy attacks and ensure that the multicall function is used securely within the intended scope of the contract's functionality.

## Assessed type

Loop

code-423n4 / 2024-06-panoptic-findings