Users can skip fee payment to Decent Bridge by bridging directly on DecentEthRouter.sol.

[c4-bot-1]

Lines of code

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DecentEthRouter.sol#L197-L215
https://github.com/code-423n4/2024-01-decent/blob/07ef78215e3d246d47a410651906287c6acec3ef/src/UTB.sol#L259-L274

Vulnerability details

Impact

Protocol will not get bridge fees.

Proof of Concept

In UTB.sol, the entry point for bridging is through bridgeAndExecute(). Users will fill in the BridgeInstructions, FeeStructure and signature as the input parameter. There are three functions called, retrieveAndCollectFees(), swapAndModifyPostBridge(), callBridge(). Note that FeeStructure is a user input.

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

Focusing on callBridge, note that the bridgeFee is calculated because calling bridge() on the BridgeAdapter.

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

This bridge fee is from the user input of bridgeAndExecute(), which means the user can set the bridge fee to zero. It wouldn't work on the Stargate bridge because stargate calculates the fees directly in the router -> pool itself.

  // request fee params from library
        SwapObj memory s = IStargateFeeLibrary(feeLibrary).getFees(poolId, _dstPoolId, _dstChainId, _from, amountSD);

https://optimistic.etherscan.io/address/0xdecc0c09c3b5f6e92ef4184125d5648a66e35298#code

However, when DecentBridgeAdapter.bridge() is called, it will eventually call router.bridgeWithPayload.

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

The DecentEthRouter.sol does not calculate any fees in the router.

Thus, users can call bridge() on DecentEthRouter.sol directly to skip payment of protocol fees and bridge fees since it has public visibility without any modifiers.

    function bridge(
        uint16 _dstChainId,
        address _toAddress,
        uint _amount,
        uint64 _dstGasForCall,
        bool deliverEth // if false, delivers WETH
>   ) public payable {
        _bridgeWithPayload(
            MT_ETH_TRANSFER,
            _dstChainId,
            _toAddress,
            _amount,
            _dstGasForCall,
            bytes(""),
            deliverEth
        );
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Recommend calculating the fees inside the router contract, like how RadiantOFT does it (before the _send() function).

Radiant line 105 (_send() function), line 115-116 (fees).
https://vscode.blockscan.com/arbitrum-one/0x41e018EB5c52d5A400fFb891B8569C8AcFe905f1

So, even if users call the DecantETHRouter directly, they will still have to pay a bridge fee.

Assessed type

Context

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #15

[c4-judge]

alex-ppg marked the issue as not a duplicate

[c4-judge]

alex-ppg marked the issue as primary issue

[alex-ppg]

The Warden has demonstrated how a different function from #590 can also have its fees and signature validation bypassed due to a lack of access control in another function of a different contract. As such, this submission is distinct and a medium-risk grade is appropriate.

[c4-judge]

alex-ppg marked the issue as selected for report

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

alex-ppg marked the issue as not selected for report

[alex-ppg]

This submission pertains to the same vulnerability as #647 and thus will be grouped there while #647 will remain as the best report of the issue.

[c4-judge]

alex-ppg marked the issue as duplicate of #647