c4-bot-6 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-decent/blob/011f62059f3a0b1f3577c8ccd1140f0cf3e7bb29/src/UTB.sol#L168-L200 https://github.com/code-423n4/2024-01-decent/blob/011f62059f3a0b1f3577c8ccd1140f0cf3e7bb29/src/UTB.sol#L259

Vulnerability details

Impact

The vulnerability is identified within the crypto protocol's UTB contract, specifically in the swapAndModifyPostBridge function, part of the UTBExecutor on the target chain. The issue arises from a lack of validation for the correctness of the output token during a token swap operation. In this scenario, an attacker can manipulate the tokenOut address, potentially swapping for a more profitable token within the UTBExecutor contract on the target chain. This manipulation can lead to the unauthorized acquisition of valuable tokens within the contract.

Proof of Concept

The vulnerability occurs in the performSwap function, where the value of tokenOut is not adequately validated. By exploiting this, an attacker can adjust the tokenOut address, allowing them to target the most lucrative tokens within the UTBExecutor contract on the target chain. Consequently, the attacker can illegitimately acquire tokens of higher value than intended during the cross-chain transfer. vulnerable function

function swapAndModifyPostBridge(
    BridgeInstructions memory instructions
)
    private
    returns (
        uint256 amount2Bridge,
        BridgeInstructions memory //@audit --> could be re-written in this way, saving gas
    )
{
    (address tokenOut, uint256 amountOut) = performSwap(
        instructions.preBridge
    );

    // Ensure that the tokenOut is a valid token address
    require(isValidToken(tokenOut), "Invalid tokenOut address");

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

    // The vulnerability lies here, where tokenOut is not adequately checked
    newPostSwapParams.tokenOut = tokenOut;

    newPostSwapParams.amountIn = IBridgeAdapter(
        bridgeAdapters[instructions.bridgeId]
    ).getBridgedAmount(amountOut, tokenOut, newPostSwapParams.tokenIn);

    instructions.postBridge.swapPayload = ISwapper(swappers[
        instructions.postBridge.swapperId
    ]).updateSwapParams(
        newPostSwapParams,
        instructions.postBridge.swapPayload
    );

    amount2Bridge = amountOut;
}

summary: attacker will set newPostSwapParams.tokenIn to like DAI(1e18 decimals) and newPostSwapParams.tokenOut will be USDC(1e6 decimals) in theory they will transfer 1 dai in the other chain,but in practise they are transfering 1e18/1e6,so 1e12 USDC (1000000000) to the target user of the other chain,because the current implementation only checks for amountOut but the tokenOut is only checked if it is the address(0) one,in all other cases the scenario shown above could be used to drain the UTBExecutor contract of the target chain.

newPostSwapParams will be this:

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

inside of the swapAndModifyPostBridge function:

function swapAndModifyPostBridge(
        BridgeInstructions memory instructions
    )
        private
        returns (
            uint256 amount2Bridge,
            BridgeInstructions memory updatedInstructions
        )
    {
        (address tokenOut, uint256 amountOut) = performSwap(
            instructions.preBridge
        );

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

        newPostSwapParams.amountIn = IBridgeAdapter(
            bridgeAdapters[instructions.bridgeId]
        ).getBridgedAmount(amountOut, tokenOut, newPostSwapParams.tokenIn);

        updatedInstructions = instructions;

        updatedInstructions.postBridge.swapPayload = ISwapper(swappers[
            instructions.postBridge.swapperId
        ]).updateSwapParams(
            newPostSwapParams,
            instructions.postBridge.swapPayload
        );

        amount2Bridge = amountOut;
    }

Tools Used

manual review

Recommended Mitigation Steps

a mapping is created that links the potential addresses of the tokens from one chain to the other,thanks to potentially an admin that manages this link between those two token addresses,by inserting,linking and authorize them in this mapping.

Assessed type

Invalid Validation

c4-pre-sort commented 8 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as duplicate of #21

c4-judge commented 8 months ago

alex-ppg marked the issue as not a duplicate

alex-ppg commented 8 months ago

The UTBExecutor contract is not meant to hold funds per the contest's scope and as such, bugs involving them are OOS as they would solely compromise accidentally sent funds.