sherlock-audit / 2023-01-uxd-judging

3 stars 1 forks source link

HonorLt - No slippage control #407

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

HonorLt

high

No slippage control

Summary

Perpetual depository uses a default value of 0 for the slippage which might incur unexpected losses for the users.

Vulnerability Detail

When placing long or short orders, the PerpDepository always passes a 0 value for the price limit, e.g.:

    function _openLong(uint256 amount)
        private
        returns (uint256, uint256)
    {
        (uint256 baseAmount, uint256 quoteAmount) = _placePerpOrder(
            amount,
            false, // isShort
            true, // isExactInput
            0 // sqrtPriceLimitX96
        );
        redeemableUnderManagement -= quoteAmount;

        return (baseAmount, quoteAmount);
    }
    function _openShort(uint256 amount)
        private
        returns (uint256, uint256)
    {
        (uint256 baseAmount, uint256 quoteAmount) = _placePerpOrder(
            amount,
            true, // short
            true, // exactInput
            0
        );
        redeemableUnderManagement += quoteAmount;
        _checkSoftCap();
        // emit event here
        return (baseAmount, quoteAmount);
    }

From Perpetual documentation, we can see that 0 means no limit: sqrtPriceLimitX96: the restriction on the ending price after the swap. 0 for no limit. This is the same as sqrtPriceLimitX96 in the Uniswap V3 contract.

Slippage control is a well-known problem in DeFi these days. With mempool lurkers and sandwich bots, you can expect it to be exploitable and harmful.

Impact

The protocol fails to protect users from possible price changes between the initiated tx and its confirmation. This might lead to extracted value from regular users.

Code Snippet

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/perp/PerpDepository.sol#L317

https://github.com/sherlock-audit/2023-01-uxd/blob/main/contracts/integrations/perp/PerpDepository.sol#L338

Tool used

Manual Review

Recommendation

It would be best if users can specify the acceptable slippage but if that's not possible, then the protocol should default to a reasonable configurable amount (e.g. 0.5%).

Duplicate of #312