Lines of code

https://github.com/code-423n4/2024-05-olas/blob/3ce502ec8b475885b90668e617f3983cea3ae29f/tokenomics/contracts/staking/WormholeTargetDispenserL2.sol#L115-L117 https://github.com/code-423n4/2024-05-olas/blob/3ce502ec8b475885b90668e617f3983cea3ae29f/tokenomics/contracts/staking/OptimismTargetDispenserL2.sol#L71-L74 https://github.com/code-423n4/2024-05-olas/blob/3ce502ec8b475885b90668e617f3983cea3ae29f/tokenomics/contracts/staking/OptimismDepositProcessorL1.sol#L131-L133 https://github.com/code-423n4/2024-05-olas/blob/3ce502ec8b475885b90668e617f3983cea3ae29f/tokenomics/contracts/staking/ArbitrumDepositProcessorL1.sol#L168-L170

Vulnerability details

Impact

Excess native tokens sent when sending messages on various depositProcessors and targetDispensers is not refunded to the sender leading to loss of funds.

Proof of Concept

Looking through the various _sendMessage functions

In WormholeTargetDispenserL2.sol

    function _sendMessage(uint256 amount, bytes memory bridgePayload) internal override {
    ......
        // Check that provided msg.value is enough to cover the cost
        if (cost > msg.value) {
            revert LowerThan(msg.value, cost);
        }

        // Send the message to L1
        uint64 sequence = IBridge(l2MessageRelayer).sendPayloadToEvm{value: cost}(uint16(l1SourceChainId),
            l1DepositProcessor, abi.encode(amount), 0, gasLimitMessage, uint16(l1SourceChainId), refundAccount);

        emit MessagePosted(sequence, msg.sender, l1DepositProcessor, amount);
    }

In ArbitrumDepositProcessorL1.sol

    function _sendMessage(
        address[] memory targets,
        uint256[] memory stakingIncentives,
        bytes memory bridgePayload,
        uint256 transferAmount
    ) internal override returns (uint256 sequence) {
        .....
        // Get the total cost
        uint256 totalCost = cost[0] + cost[1];

        // Check fot msg.value to cover the total cost
        if (totalCost > msg.value) {
            revert LowerThan(msg.value, totalCost);
        }

        if (transferAmount > 0) {
            // Approve tokens for the bridge contract
            IToken(olas).approve(l1ERC20Gateway, transferAmount);

            // Construct the data for IBridge consisting of 2 pieces:
            // uint256 maxSubmissionCost: Max gas deducted from user's L2 balance to cover base submission fee
            // bytes memory extraData: empty data
            bytes memory submissionCostData = abi.encode(maxSubmissionCostToken, "");

            // Transfer OLAS to the staking dispenser contract across the bridge
            IBridge(l1TokenRelayer).outboundTransferCustomRefund{value: cost[0]}(olas, refundAccount,
                l2TargetDispenser, transferAmount, TOKEN_GAS_LIMIT, gasPriceBid, submissionCostData);
        }

        // Assemble message data payload
        bytes memory data = abi.encodeWithSelector(RECEIVE_MESSAGE, abi.encode(targets, stakingIncentives));

        // Send a message to the staking dispenser contract on L2 to reflect the transferred OLAS amount
        sequence = IBridge(l1MessageRelayer).createRetryableTicket{value: cost[1]}(l2TargetDispenser, 0,
            maxSubmissionCostMessage, refundAccount, refundAccount, gasLimitMessage, gasPriceBid, data);
    }

In OptimismDepositProcessorL1.sol

    function _sendMessage(
        address[] memory targets,
        uint256[] memory stakingIncentives,
        bytes memory bridgePayload,
        uint256 transferAmount
    ) internal override returns (uint256 sequence) {
        .....
        // Check that provided msg.value is enough to cover the cost
        if (cost > msg.value) {
            revert LowerThan(msg.value, cost);
        }

        // Assemble data payload
        bytes memory data = abi.encodeWithSelector(RECEIVE_MESSAGE, abi.encode(targets, stakingIncentives));

        // Send message to L2
        // Reference: https://docs.optimism.io/builders/app-developers/bridging/messaging#for-l1-to-l2-transactions-1
        IBridge(l1MessageRelayer).sendMessage{value: cost}(l2TargetDispenser, data, uint32(gasLimitMessage));

        // Since there is no returned message sequence, use the staking batch nonce
        sequence = stakingBatchNonce;
    }

In OptimismTargetDispenserL2.sol

    function _sendMessage(uint256 amount, bytes memory bridgePayload) internal override {
.       .....
        // Check that provided msg.value is enough to cover the cost
        if (cost > msg.value) {
            revert LowerThan(msg.value, cost);
        }

        // Check the gas limit values for both ends
        if (gasLimitMessage < GAS_LIMIT) {
            gasLimitMessage = GAS_LIMIT;
        }

        if (gasLimitMessage > MAX_GAS_LIMIT) {
            gasLimitMessage = MAX_GAS_LIMIT;
        }

        // Assemble data payload
        bytes memory data = abi.encodeWithSelector(RECEIVE_MESSAGE, abi.encode(amount));

        // Send the message to L1 deposit processor
        // Reference: https://docs.optimism.io/builders/app-developers/bridging/messaging#for-l1-to-l2-transactions-1
        IBridge(l2MessageRelayer).sendMessage{value: cost}(l1DepositProcessor, data, uint32(gasLimitMessage));

        emit MessagePosted(0, msg.sender, l1DepositProcessor, amount);
    }

None of them refund the excess ETH back to the sender leading to loss of funds.

Tools Used

Manual code review

Recommended Mitigation Steps

Consider introducing a function to send back the msg.value minus cost to the sender.

Assessed type

ETH-Transfer

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

Excess token refund spent is not refunded back to senders in multiple places #87