DecentEthRouter.sol#_bridgeWithPayload() - Any refunded ETH (native token) will be refunded to the DecentBridgeAdapter, making them stuck

mediumCode4rena

Lines of code

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L171

Vulnerability details

Impact

The current flow of swapping and bridging tokens using the DecentBridgeAdapter looks like so:

bridgeAndExecute inside UTB is called, passing in the bridgeId of the DecentBridgeAdapter.

jsx📋
function bridgeAndExecute(
        BridgeInstructions calldata instructions,
        FeeStructure calldata fees,
        bytes calldata signature
    )
        public
        payable
        retrieveAndCollectFees(fees, abi.encode(instructions, fees), signature)
        returns (bytes memory)
    {
        (
            uint256 amt2Bridge,
            BridgeInstructions memory updatedInstructions
        ) = swapAndModifyPostBridge(instructions);
        return callBridge(amt2Bridge, fees.bridgeFee, updatedInstructions);
    }

This then makes a call to callBridge, which will call bridge on the DecentBridgeAdapter.

jsx📋
function callBridge(
        uint256 amt2Bridge,
        uint bridgeFee,
        BridgeInstructions memory instructions
    ) private returns (bytes memory) {
        bool native = approveAndCheckIfNative(instructions, amt2Bridge);
        return
            IBridgeAdapter(bridgeAdapters[instructions.bridgeId]).bridge{
                value: bridgeFee + (native ? amt2Bridge : 0)
            }(
                amt2Bridge,
                instructions.postBridge,
                instructions.dstChainId,
                instructions.target,
                instructions.paymentOperator,
                instructions.payload,
                instructions.additionalArgs,
                instructions.refund
            );
    }

DecentBridgeAdapter then makes a call to the bridgeWithPayload inside DecentEthRouter.

jsx📋
function bridge(
        uint256 amt2Bridge,
        SwapInstructions memory postBridge,
        uint256 dstChainId,
        address target,
        address paymentOperator,
        bytes memory payload,
        bytes calldata additionalArgs,
        address payable refund
    ) public payable onlyUtb returns (bytes memory bridgePayload) {
        require(
            destinationBridgeAdapter[dstChainId] != address(0),
            string.concat("dst chain address not set ")
        );

        uint64 dstGas = abi.decode(additionalArgs, (uint64));

        bridgePayload = abi.encodeCall(
            this.receiveFromBridge,
            (postBridge, target, paymentOperator, payload, refund)
        );

        SwapParams memory swapParams = abi.decode(
            postBridge.swapPayload,
            (SwapParams)
        );

        if (!gasIsEth) {
            IERC20(bridgeToken).transferFrom(
                msg.sender,
                address(this),
                amt2Bridge
            );
            IERC20(bridgeToken).approve(address(router), amt2Bridge);
        }

       
        router.bridgeWithPayload{value: msg.value}(
            lzIdLookup[dstChainId],
            destinationBridgeAdapter[dstChainId],
            swapParams.amountIn,
            false,
            dstGas,
            bridgePayload
        );
    }

bridgeWithPayoad calls the internal function _bridgeWithPayload which starts the call to LZ and the bridging process itself.

jsx📋
function _bridgeWithPayload(
        uint8 msgType,
        uint16 _dstChainId,
        address _toAddress,
        uint _amount,
        uint64 _dstGasForCall,
        bytes memory additionalPayload,
        bool deliverEth
    ) internal {
        (
            bytes32 destinationBridge,
            bytes memory adapterParams,
            bytes memory payload
        ) = _getCallParams(
                msgType,
                _toAddress,
                _dstChainId,
                _dstGasForCall,
                deliverEth,
                additionalPayload
            );

        ICommonOFT.LzCallParams memory callParams = ICommonOFT.LzCallParams({
            refundAddress: payable(msg.sender), //@audit-issue all refunded tokens will be sent to the DecentBridgeAdapter
            zroPaymentAddress: address(0x0),
            adapterParams: adapterParams
        });

        uint gasValue;
        if (gasCurrencyIsEth) {
            weth.deposit{value: _amount}();
            gasValue = msg.value - _amount;
        } else {
            weth.transferFrom(msg.sender, address(this), _amount);
            gasValue = msg.value;
        }

        dcntEth.sendAndCall{value: gasValue}(
            address(this), // from address that has dcntEth (so DecentRouter)
            _dstChainId,
            destinationBridge, // toAddress
            _amount, // amount
            payload, //payload (will have recipients address)
            _dstGasForCall, // dstGasForCall
            callParams // refundAddress, zroPaymentAddress, adapterParams
        );
    }

When we are using LZ, we have to specify LzCallParams. The struct holds several things, but importantly it holds the refundAddress

jsx📋
ICommonOFT.LzCallParams memory callParams = ICommonOFT.LzCallParams({
            refundAddress: payable(msg.sender),
            zroPaymentAddress: address(0x0),
            adapterParams: adapterParams
        });

You'll notice that the refundAddress is specified as msg.sender, in this case msg.sender is the DecentBridgeAdapter since that's the address that made the call to DecentEthRouter.

The refundAddress is used for refunding any excess native tokens (in our case) that are sent to LZ in order to pay for the gas. The excess will be refunded on the source chain.

Basically if you send 0.5 ETH to LZ for gas and LZ only needs 0.1ETH, then 0.4ETH will be sent to the refundAddress.

The problem here is, that the DecentBridgeAdapter has no way of retrieving the funds, as it doesn't implement any withdraw functionality whatsoever.

The protocol team even stated in the README.

Fund Accumulation: Other than the UTBFeeCollector, and DcntEth, the contracts are not intended to hold on to any funds or unnecessary approvals. Any native value or erc20 flowing through the protocol should either get delivered or refunded.

This bug clearly violates what the protocol team expects.

Proof of Concept

Example:

1. Alice wants to swap and bridge some tokens from Ethereum to Arbitrum.
2. For simplicity we'll assume that LZ will need 0.1 ETH in order to pay for gas fees.
3. Alice sends 0.5ETH instead.
4. The flow of functions is executed and the extra 0.4 ETH is refunded from LZ to refundAddress, which is set to msg.sender, which is DecentBridgeAdapter.
5. Alice loses here 0.4 ETH forever, as there is no way to withdraw and send the ETH from the DecentBridgeAdapter to Alice.

Tools Used

Manual Review

Recommended Mitigation Steps

The user specifies a refund when calling bridgeAndExecute inside UTB. Use the address that the user specifies instead of msg.sender.

Assessed type

ETH-Transfer