sherlock-audit / 2023-01-uxd-judging

3 stars 1 forks source link

Jeiwan - Wrong fee calculation when opening a short position on Perpetual Protocol #377

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

Jeiwan

medium

Wrong fee calculation when opening a short position on Perpetual Protocol

Summary

Total fees paid is calculated incorrectly in PerpDepository due to market fees being used instead of Uniswap fees.

Vulnerability Detail

Perpetual Protocol applies different fees depending on whether an order is long or short: Perpetual Protocol's own market fee is applied to long orders, while Uniswap swap fee is applied to short orders (Exchange.sol#L493-L524):

if (params.isBaseToQuote) {
    // short: exchangedPositionSize <= 0 && exchangedPositionNotional >= 0
    exchangedPositionSize = SwapMath
        .calcAmountScaledByFeeRatio(response.base, marketInfo.uniswapFeeRatio, false)
        .neg256();
    // due to base to quote fee, exchangedPositionNotional contains the fee
    // s.t. we can take the fee away from exchangedPositionNotional
    exchangedPositionNotional = response.quote.toInt256();
} else {
    // long: exchangedPositionSize >= 0 && exchangedPositionNotional <= 0
    exchangedPositionSize = response.base.toInt256();

    // scaledAmountForUniswapV3PoolSwap is the amount of quote token to swap (input),
    // response.quote is the actual amount of quote token swapped (output).
    // as long as liquidity is enough, they would be equal.
    // otherwise, response.quote < scaledAmountForUniswapV3PoolSwap
    // which also means response.quote < exact input amount.
    if (params.isExactInput && response.quote == scaledAmountForUniswapV3PoolSwap) {
        // NOTE: replayResponse.fee might have an extra charge of 1 wei, for instance:
        // Q2B exact input amount 1000000000000000000000 with fee ratio 1%,
        // replayResponse.fee is actually 10000000000000000001 (1000 * 1% + 1 wei),
        // and quote = exchangedPositionNotional - replayResponse.fee = -1000000000000000000001
        // which is not matched with exact input 1000000000000000000000
        // we modify exchangedPositionNotional here to make sure
        // quote = exchangedPositionNotional - replayResponse.fee = exact input
        exchangedPositionNotional = params.amount.sub(replayResponse.fee).toInt256().neg256();
    } else {
        exchangedPositionNotional = SwapMath
            .calcAmountScaledByFeeRatio(response.quote, marketInfo.uniswapFeeRatio, false)
            .neg256();
    }
}

However, getExchangeFeeWad of PerpDepository uses market fees in both long and short orders (PerpDepository.sol#L370-L371, PerpDepository.sol#L794-L797)

Impact

totalFeesPaid in PerpDepository is calculated incorrectly. All calculations that account total fees paid by a PerpDepository will be wrong.

Code Snippet

PerpDepository.sol#L795-L796

Tool used

Manual Review

Recommendation

When calculating total fees paid, consider applying Perpetual Protocol market fees to long orders and Uniswap swap fees to short orders.

Duplicate of #271