sherlock-audit / 2023-12-dodo-gsp-judging

6 stars 5 forks source link

Angry_Mustache_Man - Lack of Proper Slippage Controls can cause freezing of funds during High Volatility times #169

Closed sherlock-admin closed 10 months ago

sherlock-admin commented 10 months ago

Angry_Mustache_Man

medium

Lack of Proper Slippage Controls can cause freezing of funds during High Volatility times

Summary

According to Docs, "k(or state K)" is used as Slippage factor for the protocol but it is hardcoded to a specific value which becomes ineffective in times of high volatility.

Vulnerability Detail

Docs of PMM algorithm given by the Protocol Team describes "k" as the slippage factor as can be seen here: https://docs.dodoex.io/en/product/pmm-algorithm/details-about-pmm The same algorithm is used for the calculation of the price of Base/Quote in GSPTrader.sol, i.e., when trader has to sell base token he would use GSPTrader.sol#sellBase to sell it which has the call flow as GSPTrader.sol#sellBase-> GSPTrader.sol#querySellBase -> PMMPricing.sol#sellBaseToken -> PMMPricing.sol#_ROneSellQuoteToken -> DODOMath#_SolveQuadraticFunctionForTrade [for state.R == RState.ONE] , using the PMM algorithm with the formula same as defined in the algorithm i.e.,

 i*deltaB = (Q2-Q1)*(1-k+kQ0^2/Q1/Q2)

where k stands for slippage factor. The problem here is that , at the times of high volatility, the Trader might not be able to get their desired amount of Quote tokens for the supplied base tokens as the slippage factor here is Hardcoded(and there is no way to change the slippage factor[k] but similar functions have been added for other important variable as i[is the initial "guide price"] at GSPvault.sol#adjustPrice). Hardcoded slippage causes freezing of User funds and the User might not be able to sell their Base Tokens due to this condition in DODOMath.sol: https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/lib/DODOMath.sol#L186C7-L191C6

  uint256 V2 = DecimalMath.divCeil(numerator, denominator);
        if (V2 > V1) {
            return 0;
        } else {
            return V1 - V2;
        }
    }

where if the V2 calculated is not big enough, then it might not be able to transfer funds to the User as the value is passed back onto the GSPTrader.sol#sellBase will be 0. https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPTrader.sol#L49C7-L49C51

   _transferQuoteOut(to, receiveQuoteAmount);

Impact

Freezing of funds and preventing Users from selling their Base/Quote token.

Code Snippet

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/lib/DODOMath.sol#L91C4-L192C2

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/lib/DODOMath.sol#L186C7-L191C6

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPTrader.sol#L49C7-L49C51

Tool used

Manual Review

Recommendation

Add a function in GSPVault.sol , similar to adjustPrice function to adjust the slippage factor in times of high volatility.

nevillehuang commented 10 months ago

Invalid, k is not used as slippage control, but rather a factor used to determine curve behavior related to price. Same logic is implemented in DSP, where k is intended to be immutable upon deployment.