Decent - dutra's results

Lines of code

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

Vulnerability details

Impact

The lack of access control on setRouter allows an attacker to change the address of the router variable.

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

Because of this, the mint and burn functions can be called by anyone who sets the router, bypassing the modifier onlyRouter.

    function mint(address _to, uint256 _amount) public onlyRouter {
        _mint(_to, _amount);
    }

    function burn(address _from, uint256 _amount) public onlyRouter {
        _burn(_from, _amount);
    }

    modifier onlyRouter() {
        require(msg.sender == router);
        _;
    }

Proof of Concept

Put this test file inside the tests on src/lib/decent-bridge

Then run forge test --mt testAttack

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

import {ERC20} from "solmate/tokens/ERC20.sol";
import {UTB, SwapInstructions, SwapAndExecuteInstructions, FeeStructure} from "../src/UTB.sol";
import {UTBExecutor} from "../src/UTBExecutor.sol";
import {UniSwapper} from "../src/swappers/UniSwapper.sol";
import {SwapParams} from "../src/swappers/SwapParams.sol";
import {XChainExactOutFixture} from "./helpers/XChainExactOutFixture.sol";



contract UTBExactOutRoutesTest is XChainExactOutFixture {
    function setUp() public {
        src = optimism;
        dst = arbitrum;
        preSlippage = 2;
        postSlippage = 3;
        initialEthBalance = 1 ether;
        initialUsdcBalance = 10e6;
        MINT_GAS = 9e5;

        setRuntime(ENV_FORGE_TEST);
        loadAllChainInfo();
        setupUsdcInfo();
        setupWethHelperInfo();
        loadAllUniRouterInfo();
        setSkipFile(true);
        vm.label(alice, "alice");
        vm.label(bob, "bob");
    }


    function attackScenarioForSwapAndExecute() public {
        string memory chain = arbitrum;
        (
            UTB utb,
            /*UTBExecutor executor*/,
            UniSwapper swapper,
            ,
            ,

        ) = deployUTBAndItsComponents(chain);
        uint256 slippage = 1;

        address weth = getWeth(chain);
        address usdc = getUsdc(chain);

        cat = deployTheCat(chain);
        uint usdcOut = cat.price();

        (SwapParams memory swapParams, uint expected) = getSwapParamsExactOut(
            chain,
            weth,
            usdc,
            usdcOut,
            slippage
        );

        address payable refund = payable(alice);

        SwapInstructions memory swapInstructions = SwapInstructions({
            swapperId: swapper.getId(),
            swapPayload: abi.encode(swapParams, address(utb), refund)
        });

        mintWethTo(chain, alice, swapParams.amountIn);
        startImpersonating(alice);
        ERC20(weth).approve(address(utb), swapParams.amountIn);

        SwapAndExecuteInstructions
            memory instructions = SwapAndExecuteInstructions({
                swapInstructions: swapInstructions,
                target: address(0),
                paymentOperator: address(0),
                refund: refund,
                payload: abi.encodeCall(cat.mintWithUsdc, (bob))
            });

        (
            bytes memory signature,
            FeeStructure memory fees
        ) = getFeesAndSignature(instructions);

        utb.swapAndExecute(instructions, fees, signature);

        stopImpersonating();
        assertEq(cat.balanceOf(bob), 1);
        assertEq(ERC20(usdc).balanceOf(address(cat)), cat.price());
        assertEq(ERC20(weth).balanceOf(refund), swapParams.amountIn - expected);
    }


    function testSwapWethToUsdcAndMintAnNft() public {
        string memory chain = arbitrum;
        (
            UTB utb,
            /*UTBExecutor executor*/,
            UniSwapper swapper,
            ,
            ,

        ) = deployUTBAndItsComponents(chain);
        uint256 slippage = 1;

        address weth = getWeth(chain);
        address usdc = getUsdc(chain);

        cat = deployTheCat(chain);
        uint usdcOut = cat.price();

        (SwapParams memory swapParams, uint expected) = getSwapParamsExactOut(
            chain,
            weth,
            usdc,
            usdcOut,
            slippage
        );

        address payable refund = payable(alice);

        SwapInstructions memory swapInstructions = SwapInstructions({
            swapperId: swapper.getId(),
            swapPayload: abi.encode(swapParams, address(utb), refund)
        });

        mintWethTo(chain, alice, swapParams.amountIn);
        startImpersonating(alice);
        ERC20(weth).approve(address(utb), swapParams.amountIn);

        SwapAndExecuteInstructions
            memory instructions = SwapAndExecuteInstructions({
                swapInstructions: swapInstructions,
                target: address(cat),
                paymentOperator: address(cat),
                refund: refund,
                payload: abi.encodeCall(cat.mintWithUsdc, (bob))
            });

        (
            bytes memory signature,
            FeeStructure memory fees
        ) = getFeesAndSignature(instructions);

        utb.swapAndExecute(instructions, fees, signature);

        stopImpersonating();
        assertEq(cat.balanceOf(bob), 1);
        assertEq(ERC20(usdc).balanceOf(address(cat)), cat.price());
        assertEq(ERC20(weth).balanceOf(refund), swapParams.amountIn - expected);
    }

    function xChainCatMintSetup() public {
        dealTo(src, alice, initialEthBalance);
        mintUsdcTo(src, alice, initialUsdcBalance);
        mintWethTo(src, alice, initialEthBalance);
        setupXChainUTBInfra(src, dst);
        cat = deployTheCat(dst);
        catUsdcPrice = cat.price();
        catEthPrice = cat.ethPrice();
    }

    function testEth2Usdc() public {
        xChainCatMintSetup();
        (
            uint256 preExtraIn,
            uint256 postExtraIn,
            uint256 lzFees,
            uint256 amountIn
        ) = performXChainExactOutAndReceiveDecentBridge(
                getXChainCatUSDCMintScenario(address(0), bob)
            );

        assertSourceGasTokenAction(amountIn, preExtraIn, lzFees);
        assertDestinationMintAndRefund(postExtraIn);
        assertNoMoneyInDecentContractsAndCollectedFees(src, dst);
    }

    function testWeth2Usdc() public {
        xChainCatMintSetup();
        (
            uint256 preExtraIn,
            uint256 postExtraIn,
            uint256 lzFees,
            uint256 amountIn
        ) = performXChainExactOutAndReceiveDecentBridge(
                getXChainCatUSDCMintScenario(getWeth(src), bob)
            );

        assertSourceWethAction(amountIn, preExtraIn, lzFees);
        assertDestinationMintAndRefund(postExtraIn);
        assertNoMoneyInDecentContractsAndCollectedFees(src, dst);
    }

    function testUsdc2Usdc() public {
        xChainCatMintSetup();
        (
            uint256 preExtraIn,
            uint256 postExtraIn,
            uint256 lzFees,
            uint256 amountIn
        ) = performXChainExactOutAndReceiveDecentBridge(
                getXChainCatUSDCMintScenario(getUsdc(src), bob)
            );

        assertUsdcSourceActionBalances(lzFees, amountIn, preExtraIn);
        assertDestinationMintAndRefund(postExtraIn);
        assertNoMoneyInDecentContractsAndCollectedFees(src, dst);
    }

    function testEth2Weth() public {
        xChainCatMintSetup();
        (
            uint256 preExtraIn,
            uint256 postExtraIn,
            uint256 lzFees,
            uint256 amountIn
        ) = performXChainExactOutAndReceiveDecentBridge(
                getXChainCatWethMintScenario(address(0), bob)
            );

        assertSourceGasTokenAction(amountIn, preExtraIn, lzFees);
        assertDestinationMintAndRefund(postExtraIn);
        assertNoMoneyInDecentContractsAndCollectedFees(src, dst);
    }

    function testWeth2Weth() public {
        xChainCatMintSetup();
        (
            uint256 preExtraIn,
            uint256 postExtraIn,
            uint256 lzFees,
            uint256 amountIn
        ) = performXChainExactOutAndReceiveDecentBridge(
                getXChainCatWethMintScenario(getWeth(src), bob)
            );

        assertSourceWethAction(amountIn, preExtraIn, lzFees);
        assertDestinationMintAndRefund(postExtraIn);
        assertNoMoneyInDecentContractsAndCollectedFees(src, dst);
    }

    function testUsdc2Weth() public {
        xChainCatMintSetup();
        (
            uint256 preExtraIn,
            uint256 postExtraIn,
            uint256 lzFees,
            uint256 amountIn
        ) = performXChainExactOutAndReceiveDecentBridge(
                getXChainCatWethMintScenario(getUsdc(src), bob)
            );

        assertUsdcSourceActionBalances(lzFees, amountIn, preExtraIn);
        assertDestinationMintAndRefund(postExtraIn);
        assertNoMoneyInDecentContractsAndCollectedFees(src, dst);
    }

    function testEth2EthWithEthFees() public {
        feeAmount = 0.00069 ether;
        xChainCatMintSetup();
        (
            uint256 preExtraIn,
            uint256 postExtraIn,
            uint256 lzFees,
            uint256 amountIn
        ) = performXChainExactOutAndReceiveDecentBridge(
                getXChainCatEthMintScenario(address(0), bob)
            );

        assertSourceGasTokenAction(amountIn, preExtraIn, lzFees);
        assertDestinationMintAndRefund(postExtraIn);
        assertNoMoneyInDecentContractsAndCollectedFees(src, dst);
    }

    function testEth2EthWithUsdcFees() public {
        feeAmount = 0.15e6;
        feeToken = getUsdc(src);
        xChainCatMintSetup();
        (
            uint256 preExtraIn,
            uint256 postExtraIn,
            uint256 lzFees,
            uint256 amountIn
        ) = performXChainExactOutAndReceiveDecentBridge(
                getXChainCatEthMintScenario(address(0), bob)
            );

        switchTo(src);
        assertEq(usdcBalance(src, alice), initialUsdcBalance - feeAmount);
        assertSourceGasTokenAction(amountIn, preExtraIn, lzFees);
        assertDestinationMintAndRefund(postExtraIn);
        assertNoMoneyInDecentContractsAndCollectedFees(src, dst);
    }

    function testWeth2Eth() public {
        xChainCatMintSetup();
        (
            uint256 preExtraIn,
            uint256 postExtraIn,
            uint256 lzFees,
            uint256 amountIn
        ) = performXChainExactOutAndReceiveDecentBridge(
                getXChainCatEthMintScenario(getWeth(src), bob)
            );

        assertSourceWethAction(amountIn, preExtraIn, lzFees);
        assertDestinationMintAndRefund(postExtraIn);
        assertNoMoneyInDecentContractsAndCollectedFees(src, dst);
    }

    function testUsdc2EthWithUsdcFees() public {
        feeAmount = 0.15e6;
        feeToken = getUsdc(src);
        xChainCatMintSetup();
        (
            uint256 preExtraIn,
            uint256 postExtraIn,
            uint256 lzFees,
            uint256 amountIn
        ) = performXChainExactOutAndReceiveDecentBridge(
                getXChainCatEthMintScenario(getUsdc(src), bob)
            );

        assertUsdcSourceActionBalances(lzFees, amountIn, preExtraIn);
        assertDestinationMintAndRefund(postExtraIn);
        assertNoMoneyInDecentContractsAndCollectedFees(src, dst);
    }
}

Tools Used

vscode, foundry

Recommended Mitigation Steps

Implement proper access control to this function, so it can only be called by the contract's owner or the current router.

Assessed type

Access Control

Lines of code

https://github.com/code-423n4/2024-01-decent/blob/main/src/UTB.sol#L311-L319 https://github.com/code-423n4/2024-01-decent/blob/main/src/UTB.sol#L108-L124

Vulnerability details

Impact

The function UTB::receiveFromBridge() lacks access control, and its expected only to be called by the bridge adapter.

The fact that it can be called by any user can make a malicious user to bypass access control and fees to be paid by the swapAndExecute function.

UTB.sol#L311-L319

    function receiveFromBridge(
        SwapInstructions memory postBridge,
        address target,
        address paymentOperator,
        bytes memory payload,
        address payable refund
    ) public {
        _swapAndExecute(postBridge, target, paymentOperator, payload, refund);
    }

This function does not have access control, so it can be called by any user, rendering the swapAndExecute function useless, which has access control and fees to be paid: UTB.sol#L108-L124

    function swapAndExecute(
        SwapAndExecuteInstructions calldata instructions,
        FeeStructure calldata fees,
        bytes calldata signature
    )
        public
        payable
        retrieveAndCollectFees(fees, abi.encode(instructions, fees), signature)
    {
        _swapAndExecute(
            instructions.swapInstructions,
            instructions.target,
            instructions.paymentOperator,
            instructions.payload,
            instructions.refund
        );
    }

Proof of Concept

receiveFromBridge comments shows the intent of only being able to be called by the bridge adapter, and the logic is pretty straight forward of how it bypasses the fees of swapAndExecute since they execute the same thing.

Tools Used

vscode, foundry

Recommended Mitigation Steps

Allow the function UTB::receiveFromBridge() to only be called by the bridge adapter.

Assessed type

Access Control