Decent - cu5t0mpeo's results

Lines of code

https://github.com/decentxyz/decent-bridge/blob/7f90fd4489551b69c20d11eeecb17a3f564afb18/src/DcntEth.sol#L20-L22

Vulnerability details

Impact

Users can change the router of DcntEth at will. An attacker can forge a router contract and then call the mint and burn functions at will.

Proof of Concept

setRouter is implemented as follows

    function setRouter(address _router) public {
        router = _router;
    }

Through the implementation of the function, we can know that this function can be called by anyone, and the affected functions mainly include mint and burn

Tools Used

Manual Review

Recommended Mitigation Steps

Change it to the following:

    function setRouter(address _router) public onlyOwner{
        router = _router;
    }

Assessed type

Access Control

Lines of code

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

Vulnerability details

Impact

This will cause the amount to be stored in the Adapter contract, which is not in line with the sponsor's ideas, and will cause the user to lose part of the amount.

Proof of Concept

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.

But when the user calls the function _bridgeWithPayload, he sets the refundAddress of endpoint.send to msg.sender

    function _bridgeWithPayload(
        uint8 msgType,
        uint16 _dstChainId,
        address _toAddress,
        uint _amount,
        uint64 _dstGasForCall,
        bytes memory additionalPayload,
        bool deliverEth
    ) internal {
        ...

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

				...

        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 the transmitted gas cost is more than the actual usage cost, the excess amount will be returned to refundAddress, but since the caller is an adapter, the amount will be returned to the DecentBridgeAdapter contract

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

Finally, the excess fees will not be returned to the user, but will eventually remain in the DecentBridgeAdapter contract

Tools Used

Manual Review

Recommended Mitigation Steps

Added method to determine whether the adapter contract is called. If so, the refund object will be passed using account as a parameter.

Assessed type

Other