The `msg.value` - `cost` for multiple cross-chain bridges are not refunded to users

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/staking/ArbitrumDepositProcessorL1.sol#L119-L192 https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/staking/WormholeDepositProcessorL1.sol#L59-L98 https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/staking/WormholeTargetDispenserL2.sol#L89-L124

Vulnerability details

In multiple bridges, in _sendMessage there is a cost variable which is usually obtained by calling the bridge integration gas price estimator.

The problem is that when msg.value > cost, since cost is usually dynamic, the msg.value - cost will not be refunded to the transaction originator.

For instance, for the Wormhole integrator below:

WormholeTargetDispenserL2.sol#L89-L124

    function _sendMessage(uint256 amount, bytes memory bridgePayload) internal override {
        // Check for the bridge payload length
        if (bridgePayload.length != BRIDGE_PAYLOAD_LENGTH) {
            revert IncorrectDataLength(BRIDGE_PAYLOAD_LENGTH, bridgePayload.length);
        }

        // Extract refundAccount and gasLimitMessage from bridgePayload
        (address refundAccount, uint256 gasLimitMessage) = abi.decode(bridgePayload, (address, uint256));
        // If refundAccount is zero, default to msg.sender
        if (refundAccount == address(0)) {
            refundAccount = msg.sender;
        }

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

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

        // Get a quote for the cost of gas for delivery
        (uint256 cost, ) = IBridge(l2MessageRelayer).quoteEVMDeliveryPrice(uint16(l1SourceChainId), 0, gasLimitMessage);

        // 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);
    }

A cost is returned from quoteEVMDeliveryPrice that reflects the gas cost of executing the cross-chain transaction for the given gas limit of gasLimitMessage.

When this sendPayloadToEvm is called, cost amount will be sent for the cross-chain transaction, but the msg.value - cost will remain stuck and not refunded to the user.

Affects multiple areas of the codebase, see links to affected code.

Impact

msg.value - cost will remain stuck and not refunded to the user.

Tools Used

Manual

Recommended Mitigation Steps

Refund msg.value - cost to tx.origin

Assessed type

Other

[c4-sponsor]

kupermind (sponsor) confirmed

[c4-judge]

0xA5DF marked the issue as selected for report

[c4-judge]

0xA5DF marked the issue as satisfactory

[0xEVom]

@0xA5DF I reported this in my QA report as L-03, would you consider upgrading it to a duplicate?

On the other hand, my finding #21 is currently marked as a duplicate of this finding, but is actually a duplicate of #5.

[0xA5DF]

Fair enough, upgraded and changed the dupe

[vsharma4394]

Fixed