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

0 stars 0 forks source link

In ArbitrumDepositProcessorL1#_sendMessage, if refundAccount is 0, it is set to Dispenser contract, but the funds can't be used #266

Closed c4-bot-8 closed 4 months ago

c4-bot-8 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/staking/ArbitrumDepositProcessorL1.sol#L136 https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/staking/ArbitrumDepositProcessorL1.sol#L182 https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/staking/ArbitrumDepositProcessorL1.sol#L190

Vulnerability details

Impact

There's no logic to spend funds in the Dispenser alias on Arbitrum. Setting the recipient of a createRetryableTicket call to Dispenser contract would lead to loss of the refund

Proof of Concept

In ArbitrumDepositProcessorL1#_sendMessage, if refundAccount==address(0), it is set to msg.sender i.e. Dispenser contract address

function _sendMessage(
    address[] memory targets,
    uint256[] memory stakingIncentives,
    bytes memory bridgePayload,
    uint256 transferAmount
) internal override returns (uint256 sequence) {
    ...
    if (refundAccount == address(0)) {
        refundAccount = msg.sender;
    }
    ...
    sequence = IBridge(l1MessageRelayer).createRetryableTicket{
        value: cost[1]
    }(
        l2TargetDispenser,
        0,
        maxSubmissionCostMessage,
        refundAccount,
        refundAccount,
        gasLimitMessage,
        gasPriceBid,
        data
    );
}

But there is no logic to use the refund that gets sent to the Dispenser alias on Arbitrum. Hence, refunds are lost

Tools Used

Manual Review

Recommended Mitigation Steps

Assessed type

ETH-Transfer