A withdrawal DOS will occur for the maintainer if the tokens in the pool are not able to pay the accumulated maintainer fee

Summary

This Vulnerability occurs when a maintainer wants to withdraw their earned fees, they would not be able to if the number of tokens in the pool is not equal or more than the maintainer fee, especially if the tokens have been accumulating a long time for the maintainer and want to withdraw all at once.

Vulnerability Detail

This issue comes as a combination of two issues not addressed in the way the code is written, we will explore how first, the fees are accumulated for the maintainer

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

// update mt fee in quote token
        _MT_FEE_QUOTE_ = _MT_FEE_QUOTE_ + mtFee;

When You perform an action like sellBase and sellQuote the Fees Get Updated and it keeps accumulating until the maintainer claims the fee

But Lets explore one of those two trade functions to understand some decision making, on the fees issue, lets check the sellBase function as shown below

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

function sellBase(address to) external nonReentrant returns (uint256 receiveQuoteAmount) {
        uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this)) - _MT_FEE_BASE_;
        uint256 baseInput = baseBalance - uint256(_BASE_RESERVE_);
        uint256 mtFee;
        uint256 newBaseTarget;
        PMMPricing.RState newRState;
        // calculate the amount of quote token to receive and mt fee
        (receiveQuoteAmount, mtFee, newRState, newBaseTarget) = querySellBase(tx.origin, baseInput);
        // transfer quote token to recipient
        _transferQuoteOut(to, receiveQuoteAmount);
        // update mt fee in quote token
        _MT_FEE_QUOTE_ = _MT_FEE_QUOTE_ + mtFee;

Here we see that the base Balance is accounted for to know that at any time the base fee is being accounted for which will allow the maintainer will be able to pay when they choose to but when we read to the end of that snippet we find something interesting,

// update mt fee in quote token
        _MT_FEE_QUOTE_ = _MT_FEE_QUOTE_ + mtFee;

Here we see that the maintainer fee is being updated, but their is no check to know that the number of tokens in the pool is able to pay the new and updated maintainer accumulated fee, which will cause issues

Now Let's Explore our last snippet and see how the vulnerability will happen,

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

 /// @notice Maintainer withdraw mtFee, only for maintainer
    function withdrawMtFeeTotal() external nonReentrant onlyMaintainer {
        uint256 mtFeeQuote = _MT_FEE_QUOTE_;
        uint256 mtFeeBase = _MT_FEE_BASE_;
        _MT_FEE_QUOTE_ = 0;
        _transferQuoteOut(_MAINTAINER_, mtFeeQuote);
        _MT_FEE_BASE_ = 0;
        _transferBaseOut(_MAINTAINER_, mtFeeBase);
    }

As we can see the fees are transferred to the maintainer address, but the issue is that if the pool does not have that amount of token to pay this function will always revert , leading to a DOS situation

This Issue happens for both trade situation, whether sellBase and sellQuote

Impact

Withdrawal DOS situation for the Maintainer, that prevents them from withdrawing their tokens.

Code Snippet

Tool used

Manual Review

Recommendation

They are two suggestion i have for this issue

// update mt fee in quote token
        _MT_FEE_QUOTE_ = _MT_FEE_QUOTE_ + mtFee;

In that snippet a check should be implemented to require the pool contains the fee update before allowing further trade in the pool, so to know that at any time, they is always available tokens to pay the Maintainer.

The other solution is not a permanent solution, but it can help mitigate this issue and other once from the future, in the withdrawMtFeeTotal function, instead of demanding the full withdrawal of all accumulated fees, it should allow to withdraw a particular amount and the remaining fees should be updated.

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

inzinko - A withdrawal DOS will occur for the maintainer if the tokens in the pool are not able to pay the accumulated maintainer fee #154