Lines of code

Vulnerability details

Impact

There is a WithdrawFacet such that only the owner/admin can recover the lost funds in the contract. However, any user can retrieve the funds by using the swapTokensGeneric function, which might be unexpected behavior.

Proof of Concept

1. Suppose that 1000 USDC is left in the contract. Now, some random user wants to withdraw it.
2. They call the swapTokensGeneric function with corresponding parameters to swap 1000 USDC to DAI via the Uniswap V2 router.
3. The swapTokensGeneric function calls _executeSwaps, which calls LibSwap.swap. In this function, the contract checks whether LibAsset.getOwnBalance(fromAssetId) < fromAmount or not. If it has enough balance, it does not call LibAsset.transferFromERC20 to request the tokens from the caller.
4. Therefore, the user doesn’t have to send more USDC to the contract because the funds are enough. As a result, they get the swapped DAI without providing more USDC.

GenericSwapFacet.sol#L22-L44
Swapper.sol#L12-L23
LibSwap.sol#L33-L35

Recommended Mitigation Steps

If recovering funds by anyone is not the desired behavior, consider always transferring the funds from users before the first swap.

The text was updated successfully, but these errors were encountered:

All reactions