search

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

1 stars 1 forks source link

chaduke - SNst.drip() will eventually stop working due to overflow of totalSupply_ * nChi as nChi will growing exponentially large. #116

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 1 month ago

chaduke

Medium

SNst.drip() will eventually stop working due to overflow of totalSupply_ * nChi as nChi will growing exponentially large.

Summary

SNst.drip() will eventually stop working due to overflow of totalSupply_ * nChi as nChi will growing exponentially large. nChi will grows monotonically and will never decrease. Even though it is near 1 RAY in the beginning, due the exponential growth nature, eventually it will be a huge value.

Meanwhile, due to the scaling of Nst, totalSupply might reach as big as 100B there or even bigger due to inflation of Dollar. As a result, we will have an overflow issue for totalSupply_ * nChi and SNst.drip() will stop working.

The contract will stop working in 20 yrs for nsr = 1000000121979553151239153027 and total supply = 100B nst.

Root Cause

nChi will growing exponentially large. totalSupply could also be a large number.

[https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/sdai/src/SNst.sol#L214-L229]*https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/sdai/src/SNst.sol#L214-L229)

Internal pre-conditions

None

External pre-conditions

Enough time passed. and supply is also large.

Attack Path

Consider

  1. nsr =1000000001547125957863212448 (the value given in the test file)
  2. Supply = 100 * 10*9 10**18, 100B ether of NST. This number is likely bigger in the future.
  3. After 1000 years, we have Supply = 100000000000000000000000000000, 29 digits nChi = 1546318920731927181653166515234601763843338427037, 49 digts As a result, there is an overflow for Supply * nChi. the drip() function will revert and not work Our POC proves this finding.
  4. If we consider nsr = 1000000121979553151239153027 and total supply = 100 B nst, then after 20 years, drip() will not work due to overflow.

Impact

This drip() will not work eventually due to too big nChi. It might stop working after decades for larger nsr and stop working after 20 years with nsr =1000000121979553151239153027 .

PoC

riun forge test --match-test testVow1 -vv. 

// SPDX-License-Identifier: AGPL-3.0-or-later

// Copyright (C) 2021 Dai Foundation
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program.  If not, see <https://www.gnu.org/licenses/>.

pragma solidity ^0.8.21;

import "erc4626-tests/ERC4626.test.sol";
import { ERC1967Proxy } from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";

import { VatMock } from "test/mocks/VatMock.sol";
import { NstMock } from "test/mocks/NstMock.sol";
import { NstJoinMock } from "test/mocks/NstJoinMock.sol";

import { SNst } from "src/SNst.sol";

contract SNstERC4626Test is ERC4626Test {

    using stdStorage for StdStorage;

    VatMock vat;
    NstMock nst;
    NstJoinMock nstJoin;
    address vow;

    SNst sNst;

    uint256 constant private RAY = 10**27;

    function setUp() public override {
        vat = new VatMock();
        nst = new NstMock();
        nstJoin = new NstJoinMock(address(vat), address(nst));
        vow = 0xA950524441892A31ebddF91d3cEEFa04Bf454466;

        /* 
        function suck(address u, address v, uint rad) external note auth {
        sin[u] = add(sin[u], rad);
        dai[v] = add(dai[v], rad);
        vice   = add(vice,   rad);
        debt   = add(debt,   rad);
         }   
       */

        nst.rely(address(nstJoin));
        vat.suck(address(123), address(nstJoin), 100_000_000_000 * 10 ** 45);

        sNst = SNst(address(new ERC1967Proxy(address(new SNst(address(nstJoin), address(vow))), abi.encodeCall(SNst.initialize, ()))));
        console2.log("initial sNst.chi:", sNst.chi());
        vat.rely(address(sNst));

        /*
         function file(bytes32 what, uint256 data) external auth {
        if (what == "nsr") {
            require(data >= RAY, "SNst/wrong-nsr-value");
            require(rho == block.timestamp, "SNst/chi-not-up-to-date");
            nsr = data;
        } else revert("SNst/file-unrecognized-param");
        emit File(what, data);
    }
        */
        sNst.file("nsr", 1000000001547125957863212448); // or 1000000121979553151239153027

        vat.hope(address(nstJoin));

        _underlying_ = address(nst);
        _vault_ = address(sNst);
        _delta_ = 0;
        _vaultMayBeEmpty = true;
        _unlimitedAmount = false;
    }

    function printBalances(address a, string memory name) public{
        console2.log("===================================================");
        console2.log("Balances of ", name);
        console2.log("snt balance: ", sNst.balanceOf(a));
        console2.log("nst balance: ", nst.balanceOf(a));
        console2.log("vat sin: ", vat.sin(a));
        console2.log("vat dai: ", vat.dai(a));
        console2.log("uint.max: ", type(uint256).max);
        console2.log("===================================================");

    }

    function testVow1() public{ // try to overflow vow
        uint256 amount = 100 * 10**9 * 10**18; // 100 billion NST

        address Bob = makeAddr("Bob");
        deal(address(nst), Bob, amount);

        vm.startPrank(Bob);
        nst.approve(address(sNst), amount);        // nst deposit
        sNst.deposit(amount, Bob);
        vm.stopPrank();

        skip(1000 * 365 days);
        sNst.drip();

        printBalances(address(vow), "Vow");
        printBalances(address(Bob), "Bob");
        printBalances(address(sNst), "sNst");

    }

    function testMe() public{
        address Bob = makeAddr("Bob");
        deal(address(nst), Bob, 1000 ether);

        vm.startPrank(Bob);
        nst.approve(address(sNst), 10**7);
        sNst.deposit(10**7, Bob);
        vm.stopPrank();

        console2.log("test me...");
        skip(12);
        sNst.drip();

        printBalances(address(Bob), "Bob");
        printBalances(address(sNst), "sNst");

        skip(12);
        sNst.drip();

        printBalances(address(Bob), "Bob");
        console2.log("sNst.chi:", sNst.chi());

        console2.log("\n Before Bob deposit 100 ether...");
        printBalances(address(sNst), "sNst");
        printBalances(address(Bob), "Bob");

        vm.startPrank(Bob);
        nst.approve(address(sNst), 100 ether);
        sNst.deposit(100 ether, Bob);
        vm.stopPrank();

        console2.log("\n After Bob deposit 100 ether...");
        printBalances(address(sNst), "sNst");
        printBalances(address(Bob), "Bob");

        skip(2 weeks);
        console2.log("\n After 2 weeks... before Bob withdraw 100 ether of nst...");
        printBalances(address(sNst), "sNst");
        printBalances(address(Bob), "Bob");

        vm.startPrank(Bob);
        sNst.withdraw(100 ether, Bob, Bob);  // asets receiver owner
        vm.stopPrank();

        console2.log("\n After Bob withdraw 100 ether of nst...");
        printBalances(address(sNst), "sNst");
        printBalances(address(Bob), "Bob");

        vm.startPrank(Bob);
        sNst.redeem(sNst.balanceOf(Bob), Bob, Bob);  // asets receiver owner
        vm.stopPrank();

        console2.log("\n After Bob redeems the rest of shares...");
        printBalances(address(sNst), "sNst");
        printBalances(address(Bob), "Bob");

    }

    // setup initial vault state
    function setUpVault(Init memory init) public override {
        for (uint256 i = 0; i < N; i++) {
            init.share[i] %= 1_000_000_000 ether;
            init.asset[i] %= 1_000_000_000 ether;
            vm.assume(init.user[i] != address(0) && init.user[i] != address(sNst));
        }
        super.setUpVault(init);
    }

    // setup initial yield
    function setUpYield(Init memory init) public override {
        vm.assume(init.yield >= 0);
        init.yield %= 1_000_000_000 ether;
        uint256 gain = uint256(init.yield);

        uint256 supply = sNst.totalSupply();
        if (supply > 0) {
            uint256 nChi = gain * RAY / supply + sNst.chi();
            uint256 chiRho = (block.timestamp << 192) + nChi;
            vm.store(
                address(sNst),
                bytes32(uint256(5)),
                bytes32(chiRho)
            );
            assertEq(uint256(sNst.chi()), nChi);
            assertEq(uint256(sNst.rho()), block.timestamp);
            vat.suck(address(sNst.vow()), address(this), gain * RAY);
            nstJoin.exit(address(sNst), gain);
        }
    }

}

Mitigation

Not sure how to fix this.

telome commented 1 month ago

The provided nsr value is unrealistically large (1000000121979553151239153027 corresponds to a 4584% APY). For reference, our init script doesn't allow a larger than 100% APY, which, if it were to be used, would obviously not be sustained over year-long periods). As per the rules, "Governance configurations are assumed to be set with extreme care." and unrealistic or malicious governance configurations are out of scope.