search

code-423n4 / 2024-01-decent-findings

3 stars 3 forks source link

User can take out all the tokens from Uniswapper.sol at any time #730

Closed c4-bot-3 closed 8 months ago

c4-bot-3 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L123-L141 https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L143-L169

Vulnerability details

Impact

The malicious user can take out all the tokens from UniSwapper.sol using the functions swapExactOut() and swapExactIn()

Proof of Concept

The functions swapExactOut() and swapExactIn() have the logic to send the path of swapping tokens to the swap_router and then sending the tokens to the recipient. But the functions miss the logic for validating that swapParams.tokenOut should be same as the last token of the path provided to the swap_router. This can lead to the swap_router swapping to token(specified by the path) other than swapParams.tokenOut.

IV3SwapRouter.ExactInputParams memory params = IV3SwapRouter
            .ExactInputParams({
                path: swapParams.path,
                recipient: address(this),
                amountIn: swapParams.amountIn,
                amountOutMinimum: swapParams.amountOut
            });

        IERC20(swapParams.tokenIn).approve(uniswap_router, swapParams.amountIn);
        amountOut = IV3SwapRouter(uniswap_router).exactInput(params);

        _sendToRecipient(receiver, swapParams.tokenOut, amountOut);

The malicious user can set the swapParams.tokenOut to be the token that they want to steal from Uniswapper.sol and the path so that they can maximize the number of tokens that are coming after the swap.

The amountOut number of swapParams.tokenOut tokens are send to the reciever(user).

Tools Used

Manual Review

Recommended Mitigation Steps

Consider setting these functionsswapExactOut() and swapExactIn() as internal since these are called by the function swap() which also validates the input params.

Assessed type

Other

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 #30

raymondfam commented 8 months ago

Readme: 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.

c4-judge commented 8 months ago

alex-ppg marked the issue as unsatisfactory: Out of scope