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

3 stars 3 forks source link

Malicious user can take out all the funds from UniSwapper.sol if present #702

Closed c4-bot-10 closed 8 months ago

c4-bot-10 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-decent/blob/main/src/swappers/UniSwapper.sol#L100-L121

Vulnerability details

Impact

Malicious user can take out all tokens from UniSwapper.sol using the function swapNoPath()

Proof of Concept

Initially, the public function swapNoPath() lacks input validation.

A potential security concern arises when a malicious user, having configured the swapParams.direction as SwapDirection.EXACT_IN, invokes the function. Subsequently, the function transfers the specified amount of tokens (swapParams.amountIn) to itself, as facilitated by _receiveAndWrapIfNeeded(). Following this, an equal quantity of swapParams.tokenOut tokens is then transferred to the designated receiver. This is seen in this following snippet

        uint amt2Recipient = swapParams.direction == SwapDirection.EXACT_OUT
            ? swapParams.amountOut
            : swapParams.amountIn;

        _sendToRecipient(receiver, swapParams.tokenOut, amt2Recipient);

The malicious user can create their own ERC20 token and then call this function while setting swapParams.amountOut as the token they want to steal and swapParams.amountIn as their own built ERC20 token. And will steal all the tokens present in this address.

Tools Used

Manual Review

Recommended Mitigation Steps

This function is supposed to be internal and not public since it is not validating any inputs and is called by the function swap()

Assessed type

ERC20

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

c4-judge commented 8 months ago

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