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

2 stars 2 forks source link

Frontrunning Vulnerability in Multicall #550

Closed c4-bot-10 closed 2 months ago

c4-bot-10 commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/multicall/Multicall.sol#L12

Vulnerability details

Impact

The Multicall smart contract exposes users to potential financial risks due to frontrunning vulnerabilities. As the contract batches multiple function calls into a single transaction, malicious actors can observe the transaction prior to its confirmation and execute their own transactions that capitalize on knowing the outcomes or intentions of the batched calls. This can be particularly detrimental in scenarios involving financial transactions, trading, or other critical operations that rely on the current state of the blockchain being unchanged. Users may experience financial losses, or their intended transactions may be preempted or rendered less effective.

Proof of Concept

Loc

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/multicall/Multicall.sol#L12

function multicall(bytes[] calldata data) public payable returns (bytes[] memory results) {
        results = new bytes[](data.length);
        for (uint256 i = 0; i < 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;
            }
        }
    }

Steps to Reproduce:

  1. Prepare a multicall transaction with multiple calls that will predictably alter contract state or market conditions.
  2. Broadcast the transaction to the network.
  3. An attacker observes the transaction in the mempool and crafts a transaction that would benefit from the anticipated state change.
  4. The attacker submits their transaction with a higher gas fee to ensure it is mined first.

Tools Used

Manual

Recommended Mitigation Steps

Implement a commit-reveal scheme to obscure the intentions of transactions until after they are finalized.

Assessed type

Other

c4-judge commented 2 months ago

Picodes marked the issue as unsatisfactory: Invalid