Lines of code

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/main/src/bridge/SequencerInbox.sol#L396 https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/main/src/bridge/SequencerInbox.sol#L416 https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/main/src/libraries/GasRefundEnabled.sol#L24

Vulnerability details

The SequencerInbox contract is designed to accepts batches from the sequencer and adds them to the rollup inbox. There are various functions inside the contract that use the refundsGas modifier located in the GasRefundEnabled contract. At the beginning of the function, the startGasLeft value is calculated as the amount of gas that the will be approximately spent on the transaction initialization. At the end of the function, the total consumed gas is calculated as startGasLeft - gasleft() and the appropriate refund is sent to the caller.

An attacker could manipulate the calldata returned by the function to increase the refund amount while spending less gas than what is calculated by the contract. As a result, the refund amount would be higher than the gas amount spent by the caller.

Quoting the Natspec /// @dev this refunds the sender for execution costs of the tx /// calldata costs are only refunded if msg.sender == tx.origin to guarantee the value refunded relates to charging /// for the tx.input. this avoids a possible attack where you generate large calldata from a contract and get over-refunded

But in the scenario where the msg.sender == tx.origin, the user might manipulate the calldata returned to refund more than the actual spent on the transaction.

Proof of Concept

Initially the startGasLeft is taken into account before executing the desired function. After the execution of the desired function, the startGasLeft is increased according to the calldata returned from the contract.

File: GasRefundEnabled.sol

     modifier refundsGas(IGasRefunder gasRefunder, IReader4844 reader4844) {                                      
        uint256 startGasLeft = gasleft();
        _;
        if (address(gasRefunder) != address(0)) {
            uint256 calldataSize = msg.data.length;
            uint256 calldataWords = (calldataSize + 31) / 32;
            // account for the CALLDATACOPY cost of the proxy contract, including the memory expansion cost
            startGasLeft += calldataWords * 6 + (calldataWords**2) / 512; 
         ...

Later in the modifier, the startGasLeft is again increased.

   startGasLeft +=
         (dataHashes.length * gasPerBlob * blobBasefee) /
         block.basefee;

Even though the comments says that the caller has to be equal to tx.origin to get refunded, there is still no guarantee that the caller isn't malicious & executes the functions addSequencerL2BatchFromBlobs() or addSequencerL2BatchFromBlobsDelayProof() to profit themselves.

Even if the caller is not malicious, then too there is no guarantee that the calldata might not have additional data attached to it which would result in the same scenario as described above.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a check to limit the length of the calldata so that the startGasLeft doesn't go beyond a limit & the right amount of gas is refunded to the caller.

Assessed type

Invalid Validation

code-423n4 / 2024-05-arbitrum-foundation-validation