code-423n4 / 2024-05-olas-findings

13 stars 4 forks source link

Dispenser is unable to use excess gas refund for Arbitrum bridging #80

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-olas/blob/3ce502ec8b475885b90668e617f3983cea3ae29f/tokenomics/contracts/staking/ArbitrumDepositProcessorL1.sol#L134-L137

Vulnerability details

Relevant code: ArbitrumDepositProcessorL1::_sendMessage

Description

The Dispenser contract is responsible for distributing token incentives across Staking contracts deployed on various blockchains. Every chain has a corresponding depositProcessor contract, which facilitates the bridging and handles chain-specific transfers/messaging:

address stakingTargetEVM = address(uint160(uint256(stakingTarget)));
IDepositProcessor(depositProcessor).sendMessage{value:msg.value}(stakingTargetEVM, stakingIncentive,
        bridgePayload, transferAmount);

In every IDepositProcessor, the only contract able to call sendMessage is the Dispenser:

if (msg.sender != l1Dispenser) {
    revert ManagerOnly(l1Dispenser, msg.sender);
}

When bridging funds to Arbitrum, one of the parameters encoded in the bridgePayload is refundAccount. This refund address is going to receive all of the excess gas that was sent during bridging. Here's the Arbitrum natspec about the bridging function:

/**
 * @notice Deposit ERC20 token from Ethereum into Arbitrum. If L2 side hasn't been deployed yet, includes name/symbol/decimals data for initial L2 deploy. Initiate by GatewayRouter.
 * @dev L2 address alias will not be applied to the following types of addresses on L1:
 *      - an externally-owned account
 *      - a contract in construction
 *      - an address where a contract will be created
 *      - an address where a contract lived, but was destroyed
 * @param _l1Token L1 address of ERC20
 * @param _refundTo Account, or its L2 alias if it have code in L1, to be credited with excess gas refund in L2
 * @param _to Account to be credited with the tokens in the L2 (can be the user's L2 account or a contract), not subject to L2 aliasing
              This account, or its L2 alias if it have code in L1, will also be able to cancel the retryable ticket and receive callvalue refund
 * @param _amount Token Amount
 * @param _maxGas Max gas deducted from user's L2 balance to cover L2 execution
 * @param _gasPriceBid Gas price for L2 execution
 * @param _data encoded data from router and user
 * @return res abi encoded inbox sequence number
 */
//  * @param maxSubmissionCost Max gas deducted from user's L2 balance to cover base submission fee
function outboundTransferCustomRefund(
    address _l1Token,
    address _refundTo,
    address _to,
    uint256 _amount,
    uint256 _maxGas,
    uint256 _gasPriceBid,
    bytes calldata _data
) public payable virtual override returns (bytes memory res)

In case such address is not given in the payload, a default address is set by the ArbitrumDepositProcessorL1:

// If refundAccount is zero, default to msg.sender
if (refundAccount == address(0)) {
    refundAccount = msg.sender;
}

The issue with Dispenser being the refundAccount is that the gas refund needs to be done from the L2 side and Dispenser is unable to use it.

Root Cause

Refunds from excess gas are sent to L2 aliased address without any way to use them.

Impact

Gas refunds are lost when refundAccount == Dispenser. Thus, the contract would lose value under certain conditions. In a protocol which depends on bridging tokens, therefore paying gas fees regularly, these refunds will accrue to a substantial amount over time.

Suggested Mitigation

The default refund address should be configurable and deployed on L2. The best candidate would be ArbitrumTargetDispenserL2.

Assessed type

Other

c4-judge commented 4 months ago

0xA5DF marked the issue as satisfactory