sherlock-audit / 2023-01-uxd-judging

3 stars 1 forks source link

minhtrng - No input validation for swap parameters #415

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

minhtrng

medium

No input validation for swap parameters

Summary

The PerpDepository.rebalance function allows the caller to determine the slippage protection parameters for the swap that will be performed on Uniswap. This could be potentially used to leak value from the protocol or users.

Vulnerability Detail

The PerpDepository.rebalance function has the following function parameters:

    function rebalance(
        ...
        uint256 amountOutMinimum,
        uint160 sqrtPriceLimitX96,
        uint24 swapPoolFee,
        address account

These will be used to perform a spot market swap (on Uniswap) like this:

SwapParams memory params = SwapParams({
    ...
    amountOutMinimum: amountOutMinimum,
    sqrtPriceLimitX96: sqrtPriceLimitX96,
    poolFee: swapPoolFee
});
uint256 quoteAmountOut = spotSwapper.swapExactInput(params);

Since the values are user input, amountOutMinimum and sqrtPriceLimitX96 can be set to allow a 100% slippage trade, while poolFee can be used to chose the pool with least liquidity (and hence highest trade impact). This can be abused in a sort of sandwich-attack that looks roughly like this:

1) Exploiter performs a spot trade to push up the price up (potentially involving a flash loan) on the lowest liquidity pool available for the corresponding combination of asset and quote. 2) Performs the rebalance with bad slippage parameters and a poolFee value that selects the pool from step 1 as described above, which pushes the price further up while only returning a minimal amount of tokens. 3) Exploiter unwinds the trade with profit.

The shortfall has to be covered by the account parameter, which could for example be the insurance-address if the approval and call to depositInsurance dont happen atomically and the former is frontrun by the exploiter. This would lead to a loss of funds for the protocol.

Impact

Loss of funds to users or the protocol (depending on who has given allowance to the PerpDepository)

Code Snippet

https://github.com/sherlock-audit/2023-01-uxd/blob/217e3a22156dc735f9632aecf01775d295b158f3/contracts/integrations/perp/PerpDepository.sol#L446-L462

https://github.com/sherlock-audit/2023-01-uxd/blob/217e3a22156dc735f9632aecf01775d295b158f3/contracts/integrations/perp/PerpDepository.sol#L499-L516

Tool used

Manual Review

Recommendation

Attempt to validate the input for sensible slippage values (e.g. use TWAP and cap the price limit some percentages above/below that) and maybe even determine the pool with the highest liquidity automatically.

Duplicate of #192