Overflow risk not handled in SNst::drip, which might DOS SNst

Summary

SNst::drip is at risk of revert caused by a potential unhandled overflow condition. This results in key functions such deposit, withdraw, redeem DOS.

Vulnerability Detail

In SNst::drip, interest rate (nsr) is in RAY precision and compounding every second. There are two potential overflow cases that are not handled. (1) _rpow(nsr, block.timestamp - rho_) overflow Although low probability, if _rpow(nsr, block.timestamp - rho_) overflow, _rpow() will simply revert the transaction, instead of returning an overflow flag to allow proper handling.

//sdai/src/SNst.sol
    function _rpow(uint256 x, uint256 n) internal pure returns (uint256 z) {
...
                    let xxRound := add(xx, half)
                    if lt(xxRound, xx) {
 |>                       revert(0, 0)
                    }
...
                        let zx := mul(z, x)
                        if and(iszero(iszero(x)), iszero(eq(div(zx, x), z))) {
|>                         revert(0, 0)
                        }
...
                        let zxRound := add(zx, half)
                        if lt(zxRound, zx) {
|>                        revert(0, 0)
                        }

(https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/dba30d7a676c20dfed3bda8c52fd6702e2e85bb1/sdai/src/SNst.sol#L170)

(2) (totalSupply_ * nChi) overflow nChi is the new compounding accumulator which grow exponetially over time. And totalSupply_ can also grow significantly over time. When a call to drip() cause the (totalSupply_ * nChi) to overflow, it causes the same effect of drip() revert.

//sdai/src/SNst.sol

    function drip() public returns (uint256 nChi) {
...
            nChi = _rpow(nsr, block.timestamp - rho_) * chi_ / RAY;
            uint256 totalSupply_ = totalSupply;
   |>      diff = totalSupply_ * nChi / RAY - totalSupply_ * chi_ / RAY;
...

(https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/dba30d7a676c20dfed3bda8c52fd6702e2e85bb1/sdai/src/SNst.sol#L220)

Because drip() is invoked in critical functions such as depost, withdraw or redeem, drip() reverting will disable asset deposit and redeem.

//sdai/src/SNst.sol
    function withdraw(uint256 assets, address receiver, address owner) external returns (uint256 shares) {
        shares = _divup(assets * RAY, drip());
        _burn(assets, shares, receiver, owner);
    }

(https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/dba30d7a676c20dfed3bda8c52fd6702e2e85bb1/sdai/src/SNst.sol#L391)

Impact

This results in key functions such deposit, withdraw, redeem DOS.

Code Snippet

    function drip() public returns (uint256 nChi) {
        (uint256 chi_, uint256 rho_) = (chi, rho);
        uint256 diff;
        if (block.timestamp > rho_) {
            nChi = _rpow(nsr, block.timestamp - rho_) * chi_ / RAY;
            uint256 totalSupply_ = totalSupply;
            diff = totalSupply_ * nChi / RAY - totalSupply_ * chi_ / RAY;
            vat.suck(address(vow), address(this), diff * RAY);
            nstJoin.exit(address(this), diff);
            chi = uint192(nChi); // safe as nChi is limited to maxUint256/RAY (which is < maxUint192)
            rho = uint64(block.timestamp);
        } else {
            nChi = chi_;
        }
        emit Drip(nChi, diff);
    }

(https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/dba30d7a676c20dfed3bda8c52fd6702e2e85bb1/sdai/src/SNst.sol#L214-L228)

Tool used

Manual Review

Recommendation

Consider checking overflow for all conditions, and if the overflow flag is true, do not compound new interest. For example,

-    function _rpow(uint256 x, uint256 n) internal pure returns (uint256 z) {
+    function _rpow(uint256 x, uint256 n) internal pure returns (uint256 z, bool overflow) {
...
                    let xxRound := add(xx, half)
                    if lt(xxRound, xx) {
-                       revert(0, 0)
+                      overflow := 1
+                      break
...
    function drip() public returns (uint256 nChi) {
        (uint256 chi_, uint256 rho_) = (chi, rho);
        uint256 diff;
        if (block.timestamp > rho_) {
+            unchecked {
+             (uint256 multiplier, bool overflow) = _rpow(nsr, block.timestamp - rho_)
+            if (!overflow) {
+             nChi = chi * multiplier;
+             if(chi == nChi / multiplier) {
+                nChi = nChi / RAY;
...

sherlock-audit / 2024-06-makerdao-endgame-judging

branch_indigo - Overflow risk not handled in SNst::drip, which might DOS SNst #121