mrhudson890 - `SNst.drip()`'s incremental NST minting causes compounding asset shortfall and withdrawer losses

[sherlock-admin2]

mrhudson890

Medium

SNst.drip()'s incremental NST minting causes compounding asset shortfall and withdrawer losses

Summary

Precision loss in the SNst contract's drip mechanism during NST minting causes a cumulative asset shortfall for SNst holders. This results in the last withdrawer receiving fewer assets than expected and being unable to redeem all their shares. The amount is calculated in a PoC to be significant for users (above 0.5% for final value of 10K). Additionally, key ERC-4626 methods will revert, and view functions return incorrect values.

Root Cause

Using calculated chi as ratio of assets / shares (as was done in pot and sDai) requires RAD (1e45) credit tracking precision. However, available assets for withdrawals are the actual ERC-20 balances of minted NST, tracked in WAD precision (1e18). This causes the ratio of available assets to shares to be significantly different from the needed assets due to accumulated compounding precision losses.

First, it's useful to mention the precise method used in the current sDai, which uses the Pot contract's drip. In it, WAD ERC-20 assets are minted ONLY when they are exited during withdrawal (burn) - when they cease to grow with chi. This ensures exact correspondence between chi * shares (RAD precision) and vat.dai internal credit's, and the eventually minted assets. This allows the usage of chi through the contract as a share price.

However, SNst.sol:drip():

1. Calculates a difference between two rounded down divisions, which rounds down differently depending on the values of totalSupply , chi_ and nChi. The two results round down differently when divided by RAY, resulting in two intermediate WAD values. When time differences cause a small difference in chi, the difference in rounding are more significant.
2. The resulting quantity (diff) is in WAD precision, and is then immediately used to mint the assets (NST) that remain in the contract by exiting the new debt (in RAD precision) from nstJoin (into ERC-20, WAD precision).
3. Over time, the differences accumulate and compound exponentially with chi, because the now static NST amount is not adjusted with changing chi directly (with RAD precision that chi * shares requires). Instead it is adjusted indirectly, via the steps 1 and 2, ignoring any accumulated discrepancy.
4. Eventually, the previously incrementally minted assets are transferred to withdrawers during _burn.

uint256 diff = totalSupply_ * nChi / RAY - totalSupply_ * chi_ / RAY;
..
nstJoin.exit(address(this), diff);

Internal pre-conditions

• nsr value affects impact size

External pre-conditions

• Deposited NST into contracts, its amount and fluctuations affects impact size
• Frequent contract usage affects impact size

Attack Path

Scenario 1 (calculated in PoCs) - Last withdrawer loss:

1. User deposits an amount of NST into the SNst contract along with other users.
2. Time passes, during which drip() calls occur, minting assets incrementally, accumulating compounding precision errors.
3. User A, now one of the last remaining users, attempts to withdraw all their funds.
4. User A receives fewer assets than owed due to the accumulated shortfall, potentially losing a significant portion of their value.

Scenario 2 - Next depositor loss:

1. After losses accumulate as in scenario 1.
2. The last user to withdraw leaves the contract with 0 actual assets, but non-zero shares.
3. After a while, user B deposits into the contract.
4. User A now can redeem their remaining shares (which were not possible to redeem previously).
5. User B incurs the loss, since is now not able to withdraw their deposit.

Scenario 3 - ERC-4626 broken methods and views:

• Because of the mismatch between present pre-minted NST and value reported by convertToAssets and estimated by convertToShares:
  • totalAssets() is always incorrect
  • maxRedeem() / maxWithdraw() / previewRedeem() / previewWithdraw() will be incorrect for last withdrawer
  • redeem / withdraw of full balance of the values reported by the views will revert for the last withdrawer.

Scenario 4 - overminting and debt inflation:

1. An attacker can manipulate the calculation of diff and their control of totalSupply such that in diff = A - B , B is rounded down, but A is not.
2. In this situation, an attacker can overmint NST and inflate the vows debt (and possibly cause flop MKR auctions in excesss of actual need).

Impact

The last withdrawer (User A in the scenario) suffers a loss of their expected assets, which could be a substantial amount. The PoC shows values within the expected range (nsr values up to 20%) that result in shortfall of above 0.5% for final user value of 10K. Specifically, of around $80 in the sample values in the PoC.

Because the PoC are simplistic, it's possible that in a different combination of input values and dynamics, larger losses are possible as well.

Additionally, this issue compromises the contract's compliance with the ERC4626 standard, as functions like totalAssets(), maxRedeem(), and maxWithdraw() may return inaccurate values. This could lead to integration issues with other protocols expecting to use this values for withdrawals.

The problem is further exacerbated by:

1. Variable total supply, which can amplify shortfalls during low-supply periods.
2. Changing nsr values, which can compound the issue if increased during low-supply periods.
3. Potential future decreases in block times, which would increase the frequency of rounding errors.
4. Higher nsr values and longer timeframes create much larger discrepancies due to the exponential growth of chi.

This issue not only causes direct financial loss but also undermines the reliability and predictability of the SNst contract, potentially damaging user trust and protocol reputation.

PoC

Python was used for the PoC due to the need to simulate the maximum amount of drip iterations over a long period of time (this change being termed "endgame").

PoC run results:

>>> python3 src/drip_poc.py

1B sNST, 18.8% nsr
TS: $1000000000.0, dsr 1000000005481367156253786112, duration 20 years)
should be       3.0728263709631796e+28
actual          3.07282636326454e+28
shortfall wei   7.698639680199197e+19
shortfall   $   76.0
-----
1B sNST, 19% nsr
TS: $1000000000.0, dsr 1000000005668583612455321600, duration 20 years)
should be       3.470493969121155e+28
actual          3.4704939606402035e+28
shortfall wei   8.480951439259494e+19
shortfall   $   84.0
-----
1B sNST, 15% nsr
TS: $1000000000.0, dsr 1000000004659906625979547648, duration 20 years)
should be       1.78987597891315e+28
actual          1.7898759736238643e+28
shortfall wei   5.289285787616438e+19
shortfall   $   52.0
-----

# this is dss/src/drip_poc.py

RAY = 10**27
HALF_RAY = RAY // 2

def _rpow(x, n):
    if x == 0:
        return RAY if n == 0 else 0
    z = RAY if n % 2 == 0 else x
    n = n // 2
    while n > 0:
        x = (x * x + HALF_RAY) // RAY
        if n % 2 == 1:
            z = (z * x + HALF_RAY) // RAY
        n = n // 2
    return z

year = 365 * 86400
block_dur = 12

def calc_shortfall(totalSupply, dsr, duration):
    chi = RAY
    mint = 0
    # cache the per block chi multiplier
    block_mul = _rpow(dsr, block_dur)
    for i in range(0, duration // block_dur):
        # calculate `diff` wad as in drip(), `block_mul * chi // RAY` is nChi
        mint += (totalSupply * block_mul * chi // RAY // RAY) - (totalSupply * chi // RAY)
        # update chi
        chi = block_mul * chi // RAY
    # convertToAssets()
    expected_mint = totalSupply * (chi - RAY) // RAY;

    print(f'TS: ${totalSupply // 1e18}, dsr {dsr}, duration {duration // year} years)')
    print('should be      ', expected_mint)
    print('actual         ', mint)
    print('shortfall wei  ', expected_mint - mint)
    print('shortfall   $  ', (expected_mint - mint) // 1e18)
    print('-----')

if __name__ == "__main__":
    print('1B sNST, 18.8% nsr')
    calc_shortfall(1e27, 1000000005481367156253786112, 20 * year)

    print('1B sNST, 19% nsr')
    calc_shortfall(1e27, 1000000005668583612455321600, 20 * year)

    print('1B sNST, 15% nsr')
    calc_shortfall(1e27, 1000000004659906625979547648, 20 * year)

Mitigation

1. To mimic the currently used sDai, ensure accounting is done via internal credit (vat.dai) that allows exact correspondence to shares * chi (in RAD) without division before multiplication. Only use WAD when exiting into the ERC-20 during withdrawal (in _burn) which stop accumulating the nsr for that NST.

// in drip()
nChi = _rpow(nsr, block.timestamp - rho_) * chi_ / RAY;
- diff = totalSupply_ * nChi / RAY - totalSupply_ * chi_ / RAY;
- vat.suck(address(vow), address(this), diff * RAY);
- nstJoin.exit(address(this), diff);
+ vat.suck(address(vow), address(this), totalSupply_ * (nChi - chi));

// in _burn()
- nst.transfer(receiver, assets);
+ nstJoin.exit(receiver, assets);

// in _mint()
nst.transferFrom(msg.sender, address(this), assets);
+ nstJoin.join(address(this), assets);

2. Alternatively, if ERC-20 balances held in the contract are desirable, the share price should be calculated from the ERC-20 balance, and chi should not be tracked. In such an implementation, drip should calculate the mint amount from the available ERC-20 assets using the nsr. This approach can use existing ERC-4626 libraries.

[telome]

There is a bug in the python PoC. The diff calculation doesn't correctly prioritize the arithmetic operations (which are not commutative due to the use of //). That line should be: mint += (totalSupply * (block_mul * chi // RAY) // RAY) - (totalSupply * chi // RAY) (note the addition of the inner parentheses). With that line fixed, you can verify that there is no shortfall.