XDZIBEC - a ceiling division in reserve updates could lead to an imbalance in share value

[sherlock-admin2]

XDZIBEC

medium

a ceiling division in reserve updates could lead to an imbalance in share value

Summary

• In the sellShares function, we have both _BASE_TARGET_ and _QUOTE_TARGET_they get updated after a user sells their share. and we have the contract uses DecimalMath._divCeil in DecimalMath.sol contract for the calculation of the new target values, which implements a division that rounds up any fractional remainder to the next whole number. the problem is If _BASE_TARGET_ or _QUOTE_TARGET_ is updated in such a way that the rounding leads to more tokens being subtracted from the target than the actual number of tokens represented by the seller’s shares, it could create a slight imbalance in the reserve, effectively causing a loss to the seller. In a pool with many transactions occurring, this could be exploited in a way that incrementally decreases the value of shares held by LPs over time Due to _divCeil potentially rounding up during each sell transaction, extra tokens can deducted from the targets, slowly depleting the reserve in relation to the shares outstanding. This imbalance would slowly disfavor the LPs, as they might receive slightly fewer tokens than they are entitled to based on their share of the total supply due to the upward rounding of the target reserves.

Vulnerability Detail

• here is_divCeil function in the theDecimalMath.sol contract :

 function _divCeil(uint256 a, uint256 b) internal pure returns (uint256) {
        uint256 quotient = a / b;
        uint256 remainder = a - quotient * b;
        if (remainder > 0) {
            return quotient + 1;
        } else {
            return quotient;
        }
    }
}

• and here the function sellShares that contain the vulenrbale part in the GSPFunding.solcontract :

        // The target will be updated
        _BASE_TARGET_ = uint112(uint256(_BASE_TARGET_) - DecimalMath._divCeil((uint256(_BASE_TARGET_) * (shareAmount)), totalShares));
        _QUOTE_TARGET_ = uint112(uint256(_QUOTE_TARGET_) - DecimalMath._divCeil((uint256(_QUOTE_TARGET_) * (shareAmount)), totalShares));

• let's as an example where the rounding effect of _divCeil leads to the reduction of targets by more than their proportional share: let say we have :
  • Initial _BASETARGET: 10,000
  • Total supply of shares (totalShares): 1,000
  • The seller wants to sell (shareAmount): 111 shares Regular division would mean: (10,000 111) / 1000 = 1,110 But with the _divCeil function, if there’s even a slight remainder, for example: (10,001 111) / 1000 = 1,110.111 The _divCeil would round up: 1,110.111 effectively becomes 1,111 the _BASETARGET would be reduced by 1,111 instead of 1,110. Over time and many similar transactions, the base target would be lowered more than justified by the sold shares, meaning the remaining shareholders indirectly lose a small amount of their equity within the pool with each rounded transaction.

    Impact

    the issue can lead This could result in liquidity providers receiving less value when withdrawing than is correctly proportionate to their shareholdings

    Code Snippet

• https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPFunding.sol#L116C5-L118C9
• https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/lib/DecimalMath.sol#L69

  Tool used

Manual Review

Recommendation

• it's recommended to replace the ceiling division when computing the proportion of the reserves to be reduced with standard division as an example :

  BASE_TARGET = uint112(uint256(BASE_TARGET) - (uint256(BASE_TARGET) * shareAmount) / totalShares);
  QUOTE_TARGET = uint112(uint256(QUOTE_TARGET) - (uint256(QUOTE_TARGET) * shareAmount) / totalShares); 

[nevillehuang]

Invalid, difference shown in example is so small (1) that it is hard to justify any severity other than low/informational