Bandit - `newBaseTarget` is Based off Outdated State

[sherlock-admin2]

Bandit

high

newBaseTarget is Based off Outdated State

Summary

newBaseTarget is based off the state which has not had the RState change applied yet, leading to an incorrect price curve.

Vulnerability Detail

In the querySellBase function the newBaseTarget is based off the state struct stored in memory

newBaseTarget = state.B0;

defined here:

PMMPricing.PMMState memory state = getPMMState();

The problem is that the new RState is calculated in the call:

(receiveQuoteAmount, newRState) = PMMPricing.sellBaseToken(state, payBaseAmount);

but the RState change is not applied to the state struct. That means that the newBaseTarget is updated incorrectly. The change in RState is only applied after the querySellBase call in the SellBase function:

        if (_RState_ != uint32(newRState)) {    
            require(newBaseTarget <= type(uint112).max, "OVERFLOW");
            _BASE_TARGET_ = uint112(newBaseTarget);
            _RState_ = uint32(newRState);
            emit RChange(newRState);
        }

Note: This also applies to QuerySellQuote

Impact

newBaseTarget is updated incorrectly which leads to an incorrect AMM price curve.

Code Snippet

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

Tool used

Manual Review

Recommendation

Apply the change to RState first, and then retrieve the newBaseTarget from adjustedTarget

[Skyewwww]

The Rstate is updated if the Rstate changes after the execution of swap. The next time before the swap is executed, the target will be adjusted according to the current Rstate.

[nevillehuang]

Hi @Skyewwww could you point me to the logic where the swap action invokes update to the target based on current R state?

[Skyewwww]

Hi, please refer to the querySellQuote/querySellBase. The target is updated using getPMMState().

    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);
    }

    function adjustedTarget(PMMState memory state) internal pure {
        if (state.R == RState.BELOW_ONE) {
            state.Q0 = DODOMath._SolveQuadraticFunctionForTarget(
                state.Q,
                state.B - state.B0,
                state.i,
                state.K
            );
        } else if (state.R == RState.ABOVE_ONE) {
            state.B0 = DODOMath._SolveQuadraticFunctionForTarget(
                state.B,
                state.Q - state.Q0,
                DecimalMath.reciprocalFloor(state.i),
                state.K
            );
        }
    }

Hi @Skyewwww could you point me to the logic where the swap action invokes update to the target based on current R state?