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

6 stars 5 forks source link

Varun_05 - Base/Quote token reserve can become equal to zero when K=0 #157

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

Varun_05

high

Base/Quote token reserve can become equal to zero when K=0

Summary

For some liquidity pools which have LP fee rate equal to zero or to be precise less than 0.1 percent it can cause the base/quote token reserve to be equal to zero which can have a great impact for example if a new liquidity providers calls the buy shares function to buy shares and sends the tokens then buy shares function would revert and it would not be possible to refill the reserves until all the shares supply becomes equal to zero

Vulnerability Detail

According to the docs https://docs.dodoex.io/en/product/fees there are liquidity pools with lp fee rate equal to zero. So lets start with the buyShares function which as follows Note the following poc applies when k = 0

function buyShares(address to)
        external
        nonReentrant
        returns (
            uint256 shares,
            uint256 baseInput,
            uint256 quoteInput
        )
    {
        // The balance of baseToken and quoteToken should be the balance minus the fee
        uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this)) - _MT_FEE_BASE_;
        uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)) - _MT_FEE_QUOTE_;
        // The reserve of baseToken and quoteToken
        uint256 baseReserve = _BASE_RESERVE_;
        uint256 quoteReserve = _QUOTE_RESERVE_;

        // The amount of baseToken and quoteToken user transfer to GSP
        baseInput = baseBalance - baseReserve;
        quoteInput = quoteBalance - quoteReserve;

        // BaseToken should be transferred to GSP before calling buyShares
        require(baseInput > 0, "NO_BASE_INPUT");

        // Round down when withdrawing. Therefore, never be a situation occuring balance is 0 but totalsupply is not 0
        // But May Happen,reserve >0 But totalSupply = 0
        if (totalSupply == 0) {
            // case 1. initial supply
            // The shares will be minted to user
            shares = quoteBalance < DecimalMath.mulFloor(baseBalance, _I_)
                ? DecimalMath.divFloor(quoteBalance, _I_)
                : baseBalance;
            // The target will be updated
            _BASE_TARGET_ = uint112(shares);
            _QUOTE_TARGET_ = uint112(DecimalMath.mulFloor(shares, _I_));
        } else if (baseReserve > 0 && quoteReserve > 0) {
            // case 2. normal case
            uint256 baseInputRatio = DecimalMath.divFloor(baseInput, baseReserve);
            uint256 quoteInputRatio = DecimalMath.divFloor(quoteInput, quoteReserve);
            uint256 mintRatio = quoteInputRatio < baseInputRatio ? quoteInputRatio : baseInputRatio;
            // The shares will be minted to user
            shares = DecimalMath.mulFloor(totalSupply, mintRatio);

            // The target will be updated
            _BASE_TARGET_ = uint112(uint256(_BASE_TARGET_) + (DecimalMath.mulFloor(uint256(_BASE_TARGET_), mintRatio)));
            _QUOTE_TARGET_ = uint112(uint256(_QUOTE_TARGET_) + (DecimalMath.mulFloor(uint256(_QUOTE_TARGET_), mintRatio)));
        }
        // The shares will be minted to user
        // The reserve will be updated
        _mint(to, shares);
        _setReserve(baseBalance, quoteBalance);
        emit BuyShares(to, shares, _SHARES_[to]);
    }

Lets take I price = 10**18

  1. first when there is no supply of shares so totalSupply == 0
  2. User buys shares by sending 1500 base tokens and 1500 quote tokens so baseBalance = 1500 and quoteBalance = 1500 baseReserve and quoteReserve = 0 so baseInput and quoteinput = 1500
  3. Shares after calculation comes out to be shares = shares = quoteBalance < DecimalMath.mulFloor(baseBalance, I) ? DecimalMath.divFloor(quoteBalance, I) : baseBalance; so quote balance = 1500 and DecimalMath.mulFloor(basebalance=1500,I = 10**18) gives 1500 so shares = 1500
    function mulFloor(uint256 target, uint256 d) internal pure returns (uint256) {
        return target * d / (10 ** 18);
    }

    So now 1500 shares would be minted and mint function as as follows

    function _mint(address user, uint256 value) internal {
        require(value > 1000, "MINT_AMOUNT_NOT_ENOUGH");
        _SHARES_[user] = _SHARES_[user] + value;
        totalSupply = totalSupply + value;
        emit Mint(user, value);
        emit Transfer(address(0), user, value);
    }
  4. Then the following is called _setReserve(baseBalance, quoteBalance); i.e _setReserve(1500, 1500);
    function _setReserve(uint256 baseReserve, uint256 quoteReserve) internal {
        // the reserves should be less than the max uint112
        require(baseReserve <= type(uint112).max && quoteReserve <= type(uint112).max, "OVERFLOW");
        _BASE_RESERVE_ = uint112(baseReserve);
        _QUOTE_RESERVE_ = uint112(quoteReserve);
        // if _IS_OPEN_TWAP_ is true, update the twap price
        if (_IS_OPEN_TWAP_) _twapUpdate();
    }

    Now _BASERESERVE = 1500 _QUOTERESERVE = 1500

So till now _BASERESERVE = 1500 , _QUOTERESERVE = 1500, totalSupply = 1500, _BASETARGET = 1500 _QUOTETARGET = 1500

  1. Now a user might be shareholder or can be anyone calls the sell quote function and transfer 1500 quote tokens to the contract

    function sellQuote(address to) external nonReentrant returns (uint256 receiveBaseAmount) {
        uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)) - _MT_FEE_QUOTE_;
        uint256 quoteInput = quoteBalance - uint256(_QUOTE_RESERVE_);
        uint256 mtFee;
        uint256 newQuoteTarget;
        PMMPricing.RState newRState;
        // calculate the amount of base token to receive and mt fee
        (receiveBaseAmount, mtFee, newRState, newQuoteTarget) = querySellQuote(
            tx.origin,
            quoteInput
        );
        // transfer base token to recipient
        _transferBaseOut(to, receiveBaseAmount);
        // update mt fee in base token
        _MT_FEE_BASE_ = _MT_FEE_BASE_ + mtFee;
    
        // update TARGET
        if (_RState_ != uint32(newRState)) {
            require(newQuoteTarget <= type(uint112).max, "OVERFLOW");
            _QUOTE_TARGET_ = uint112(newQuoteTarget);
            _RState_ = uint32(newRState);
            emit RChange(newRState);
        }
        // update reserve
        _setReserve((_BASE_TOKEN_.balanceOf(address(this)) - _MT_FEE_BASE_), quoteBalance);
    
        emit DODOSwap(
            address(_QUOTE_TOKEN_),
            address(_BASE_TOKEN_),
            quoteInput,
            receiveBaseAmount,
            msg.sender,
            to
        );
    }

    So now quoteBalance becomes 3000 quoteinput = 1500

Then querySellQuote function is called with the values as follows querySellQuote(tx.origin,1500)

function querySellQuote(address trader, uint256 payQuoteAmount)
        public
        view
        returns (
            uint256 receiveBaseAmount,
            uint256 mtFee,
            PMMPricing.RState newRState,
            uint256 newQuoteTarget
        )
    {
        PMMPricing.PMMState memory state = getPMMState();
        (receiveBaseAmount, newRState) = PMMPricing.sellQuoteToken(state, payQuoteAmount);

        uint256 lpFeeRate = _LP_FEE_RATE_;
        uint256 mtFeeRate = _MT_FEE_RATE_;
        mtFee = DecimalMath.mulFloor(receiveBaseAmount, mtFeeRate);
        receiveBaseAmount = receiveBaseAmount
            - DecimalMath.mulFloor(receiveBaseAmount, lpFeeRate)
            - mtFee;
        newQuoteTarget = state.Q0;
    }
}

to get the state getPMMState(); is called which as follows

 function getPMMState() public view returns (PMMPricing.PMMState memory state) {
        state.i = _I_;
        state.K = _K_;
        state.B = _BASE_RESERVE_;
        state.Q = _QUOTE_RESERVE_;
        state.B0 = _BASE_TARGET_; // will be calculated in adjustedTarget
        state.Q0 = _QUOTE_TARGET_;
        state.R = PMMPricing.RState(_RState_);
        PMMPricing.adjustedTarget(state);
    }

pluggin in the values e18 = 10**18 state.i = e18; state.K = 0(for this particular case it is equal to 0 }; state.B = 1500; state.Q = 1500; state.B0 = _BASETARGET; // will be calculated in adjustedTarget state.Q0 = _QUOTETARGET; state.R = PMMPricing.RState(RState) = ONE PMMPricing.adjustedTarget(state); this doesn't do anything as intially rstate = one

Now the following function is called PMMPricing.sellQuoteToken(state, payQuoteAmount); here payQuoteAmount = 1500

  function sellQuoteToken(PMMState memory state, uint256 payQuoteAmount)
        internal
        pure
        returns (uint256 receiveBaseAmount, RState newR)
    {
        if (state.R == RState.ONE) {
            receiveBaseAmount = _ROneSellQuoteToken(state, payQuoteAmount);
            newR = RState.ABOVE_ONE;
        } else if (state.R == RState.ABOVE_ONE) {
            receiveBaseAmount = _RAboveSellQuoteToken(state, payQuoteAmount);
            newR = RState.ABOVE_ONE;
        } else {
            uint256 backToOnePayQuote = state.Q0 - state.Q;
            uint256 backToOneReceiveBase = state.B - state.B0;
            if (payQuoteAmount < backToOnePayQuote) {
                receiveBaseAmount = _RBelowSellQuoteToken(state, payQuoteAmount);
                newR = RState.BELOW_ONE;
                if (receiveBaseAmount > backToOneReceiveBase) {
                    receiveBaseAmount = backToOneReceiveBase;
                }
            } else if (payQuoteAmount == backToOnePayQuote) {
                receiveBaseAmount = backToOneReceiveBase;
                newR = RState.ONE;
            } else {
                receiveBaseAmount = backToOneReceiveBase + (
                    _ROneSellQuoteToken(state, payQuoteAmount - backToOnePayQuote)
                );
                newR = RState.ABOVE_ONE;
            }
        }
    }

As state.R == RState.ONE is true then if condition will be executed with payQuoteAmount = 1500

 if (state.R == RState.ONE) {
            receiveBaseAmount = _ROneSellQuoteToken(state, payQuoteAmount);
            newR = RState.ABOVE_ONE;

ROneSellQuoteToken(state, 1500); will be called ----------- 6th step

 function _ROneSellQuoteToken(PMMState memory state, uint256 payQuoteAmount)
        internal
        pure
        returns (
            uint256 // receiveBaseToken
        )
    {
        return
            DODOMath._SolveQuadraticFunctionForTrade(
                state.B0,
                state.B0,
                payQuoteAmount,
                DecimalMath.reciprocalFloor(state.i),
                state.K
            );
    }
Following will be returned   DODOMath._SolveQuadraticFunctionForTrade(
                state.B0,
                state.B0,
                payQuoteAmount,
                DecimalMath.reciprocalFloor(state.i),
                state.K
            );

            with the values as follows
                state.B0 = 1500
                state.B0 = 1500
                payQuoteAmount = 1500
                DecimalMath.reciprocalFloor(state.i) = e18
                state.K = 0

Now following function is called with V0 = 1500,V1 = 1500, delta = 1500,i = e18,k = 0

  function _SolveQuadraticFunctionForTrade(
        uint256 V0,
        uint256 V1,
        uint256 delta,
        uint256 i,
        uint256 k
    ) internal pure returns (uint256) {
        require(V0 > 0, "TARGET_IS_ZERO");
        if (delta == 0) {
            return 0;
        }

        if (k == 0) {
            // why v1
            return DecimalMath.mulFloor(i, delta) > V1 ? V1 : DecimalMath.mulFloor(i, delta);
        }

        if (k == DecimalMath.ONE) {
            // if k==1
            // Q2=Q1/(1+ideltaBQ1/Q0/Q0)
            // temp = ideltaBQ1/Q0/Q0
            // Q2 = Q1/(1+temp)
            // Q1-Q2 = Q1*(1-1/(1+temp)) = Q1*(temp/(1+temp))
            // uint256 temp = i.mul(delta).mul(V1).div(V0.mul(V0));
            uint256 temp;
            uint256 idelta = i * (delta);
            if (idelta == 0) {
                temp = 0;
            } else if ((idelta * V1) / idelta == V1) {
                temp = (idelta * V1) / (V0 * V0);
            } else {
                temp = delta * (V1) / (V0) * (i) / (V0);
            }
            return V1 * (temp) / (temp + (DecimalMath.ONE));
        }

        // calculate -b value and sig
        // b = kQ0^2/Q1-i*deltaB-(1-k)Q1
        // part1 = (1-k)Q1 >=0
        // part2 = kQ0^2/Q1-i*deltaB >=0
        // bAbs = abs(part1-part2)
        // if part1>part2 => b is negative => bSig is false
        // if part2>part1 => b is positive => bSig is true
        uint256 part2 = k * (V0) / (V1) * (V0) + (i * (delta)); // kQ0^2/Q1-i*deltaB
        uint256 bAbs = (DecimalMath.ONE - k) * (V1); // (1-k)Q1

        bool bSig;
        if (bAbs >= part2) {
            bAbs = bAbs - part2;
            bSig = false;
        } else {
            bAbs = part2 - bAbs;
            bSig = true;
        }
        bAbs = bAbs / (DecimalMath.ONE);

        // calculate sqrt
        uint256 squareRoot = DecimalMath.mulFloor((DecimalMath.ONE - k) * (4), DecimalMath.mulFloor(k, V0) * (V0)); // 4(1-k)kQ0^2
        squareRoot = Math.sqrt((bAbs * bAbs) + squareRoot); // sqrt(b*b+4(1-k)kQ0*Q0)

        // final res
        uint256 denominator = (DecimalMath.ONE - k) * 2; // 2(1-k)
        uint256 numerator;
        if (bSig) {
            numerator = squareRoot - bAbs;
        } else {
            numerator = bAbs + squareRoot;
        }

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

Now as k =0 following will be executed

  if (k == 0) {
            // why v1
            return DecimalMath.mulFloor(i, delta) > V1 ? V1 : DecimalMath.mulFloor(i, delta);
        }

So DecimalMath.mulFloor(i, delta) = DecimalMath.mulFloor(e18, 1500) = 1500 and V1 = 1500 so the returned value is equal to 1500

So _ROneSellQuoteToken(state, 1500) highlighted above as 6th step returns 1500 So (receiveBaseAmount, newRState) = PMMPricing.sellQuoteToken(state, payQuoteAmount); returns (1500,RState.ABOVE_ONE)

Therefore receiveBaseAmount = 1500 Now the following calculations will be done

        uint256 lpFeeRate = _LP_FEE_RATE_;
        uint256 mtFeeRate = _MT_FEE_RATE_;
        mtFee = DecimalMath.mulFloor(receiveBaseAmount, mtFeeRate);
        receiveBaseAmount = receiveBaseAmount
            - DecimalMath.mulFloor(receiveBaseAmount, lpFeeRate)
            - mtFee;
        newQuoteTarget = state.Q0;
    }

As lp fee rate is taken as zero Let's take _MT_FEERATE = 5% i.e 5e16 So mtFee = DecimalMath.mulFloor(receiveBaseAmount, mtFeeRate); = 75 receiveBaseAmount = 1500-0-75 = 1425 newQuoteTarget = state.Q0 = 1500

Back to original sellquote function

 // calculate the amount of base token to receive and mt fee
        (receiveBaseAmount, mtFee, newRState, newQuoteTarget) = querySellQuote(
            tx.origin,
            quoteInput
        );

receiveBaseAmount = 1425 mtFee = 75 newRState = above one newQuoteTarget = 1500 Then _transferBaseOut(to, receiveBaseAmount); is called i.e _transferBaseOut(to, 1425); Now initially only first shareHolder had transferred 1500 base tokens and now we have transferred 1425 so left amount of base tokens = 75 _MT_FEEBASE = _MT_FEEBASE + mtFee; = 0 + 75 therefore _MT_FEEBASE = 75 Finally we update the reserves by following _setReserve((_BASETOKEN.balanceOf(address(this)) - _MT_FEEBASE), quoteBalance); _BASETOKEN.balanceOf(address(this)) = 75 as we have transferred 1425 to the user _MT_FEEBASE = 75 quoteBalance = 3000 therefore _setReserve is called as follows _setReserve(0,3000) and set reserve function is as follows

 function _setReserve(uint256 baseReserve, uint256 quoteReserve) internal {
        // the reserves should be less than the max uint112
        require(baseReserve <= type(uint112).max && quoteReserve <= type(uint112).max, "OVERFLOW");
        _BASE_RESERVE_ = uint112(baseReserve);
        _QUOTE_RESERVE_ = uint112(quoteReserve);
        // if _IS_OPEN_TWAP_ is true, update the twap price
        if (_IS_OPEN_TWAP_) _twapUpdate();
    }

Now _BASERESERVE = 0 _QUOTERESERVE = 1500

Similarly we can prove for quote Reserve = 0

Impact

Now if the initial Shareholder hasn't sold its shares and reduced the totalSupply to zero, then nobody would be able to buy shares until base reserve becomes greater than zero i.e when base tokens are sold. This is because when buyShares function will be called neither if condition will be satisfied nor else if

 if (totalSupply == 0) {
            // case 1. initial supply
            // The shares will be minted to user
            shares = quoteBalance < DecimalMath.mulFloor(baseBalance, _I_)
                ? DecimalMath.divFloor(quoteBalance, _I_)
                : baseBalance;
            // The target will be updated
            _BASE_TARGET_ = uint112(shares);
            _QUOTE_TARGET_ = uint112(DecimalMath.mulFloor(shares, _I_));
        } else if (baseReserve > 0 && quoteReserve > 0) {
            // case 2. normal case
            uint256 baseInputRatio = DecimalMath.divFloor(baseInput, baseReserve);
            uint256 quoteInputRatio = DecimalMath.divFloor(quoteInput, quoteReserve);
            uint256 mintRatio = quoteInputRatio < baseInputRatio ? quoteInputRatio : baseInputRatio;
            // The shares will be minted to user
            shares = DecimalMath.mulFloor(totalSupply, mintRatio);

            // The target will be updated
            _BASE_TARGET_ = uint112(uint256(_BASE_TARGET_) + (DecimalMath.mulFloor(uint256(_BASE_TARGET_), mintRatio)));
            _QUOTE_TARGET_ = uint112(uint256(_QUOTE_TARGET_) + (DecimalMath.mulFloor(uint256(_QUOTE_TARGET_), mintRatio)));
        }

so shares will be equal to zero and when _mint(to,0) will be called it will revert because minimum shares minted are 1000. So this can cause DOS of buy shares function

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Introduce lp mint fe and that too greater than 0.1% or you can handle the case when base/quote token reserve = 0 just like the way you do for totalSupply == 0 case

Duplicate of #49

nevillehuang commented 5 months ago

See comments here, where pools can be unblocked at any time by admin/user syncing reserves. Admin can first withdraw maintenance fee to set respective MT_FEE to zero before performing unblocking, in that case, user would simply be losing by paying maintenance fee to try and grief pool.