In LockstakeEngine.sol, the functions wipe(), wipeAll(), and free() lack the call to jug.drip(ilk)

Summary

In LockstakeEngine.sol, the functions wipe(), wipeAll(), and free() lack the call to jug.drip(ilk). The absence of jug.drip(ilk) in wipe() and wipeAll() results in users paying fewer fees, while the absence of jug.drip(ilk) in free() causes the collateral value to fall below the debt value after users withdraw collateral, leading to a loss for the protocol.

Root Cause

https://docs.makerdao.com/smart-contract-modules/rates-module/jug-detailed-documentation From the MakerDAO documentation, we know that The primary function of the Jug smart contract is to accumulate stability fees for a particular collateral type whenever its drip() method is called. This effectively updates the accumulated debt for all Vaults of that collateral type as well as the total accumulated debt as tracked by the Vat (global) and the amount of Dai surplus (represented as the amount of Dai owned by the Vow).

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/lockstake/src/LockstakeEngine.sol#L391

    function wipe(address urn, uint256 wad) external {
        nst.transferFrom(msg.sender, address(this), wad);
        nstJoin.join(address(this), wad);
@>        (, uint256 rate,,,) = vat.ilks(ilk);
@>        uint256 dart = wad * RAY / rate;
        require(dart <= uint256(type(int256).max), "LockstakeEngine/overflow");
        vat.frob(ilk, urn, address(0), address(this), 0, -int256(dart));
        emit Wipe(urn, wad);
    }

    function wipeAll(address urn) external returns (uint256 wad) {
        (, uint256 art) = vat.urns(ilk, urn);
        require(art <= uint256(type(int256).max), "LockstakeEngine/overflow");
@>        (, uint256 rate,,,) = vat.ilks(ilk);
@>        wad = _divup(art * rate, RAY);
        nst.transferFrom(msg.sender, address(this), wad);
        nstJoin.join(address(this), wad);
        vat.frob(ilk, urn, address(0), address(this), 0, -int256(art));
        emit Wipe(urn, wad);
    }

However, in both wipe() and wipeAll(), an outdated rate is used. In wipe(), dart will increase, allowing users to repay more debt. In wipeAll(), wad will decrease, meaning that less Dai is used to repay all the debt.

The lost stability fees depend on the time since the last call to jug.drip. The longer the current time is from the last call to jug.drip, the greater the lost stability fees.

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/lockstake/src/LockstakeEngine.sol#L340

function free(address urn, address to, uint256 wad) external urnAuth(urn) returns (uint256 freed) {
        freed = _free(urn, wad, fee);
        mkr.transfer(to, freed);
        emit Free(urn, to, wad, freed);
    }

function _free(address urn, uint256 wad, uint256 fee_) internal returns (uint256 freed) {
        require(wad <= uint256(type(int256).max), "LockstakeEngine/overflow");
        address urnFarm = urnFarms[urn];
        if (urnFarm != address(0)) {
            LockstakeUrn(urn).withdraw(urnFarm, wad);
        }
        lsmkr.burn(urn, wad);
@>        vat.frob(ilk, urn, urn, address(0), -int256(wad), 0);
        vat.slip(ilk, urn, -int256(wad));
        address voteDelegate = urnVoteDelegates[urn];
        if (voteDelegate != address(0)) {
            VoteDelegateLike(voteDelegate).free(wad);
        }
        uint256 burn = wad * fee_ / WAD;
        if (burn > 0) {
            mkr.burn(address(this), burn);
        }
        unchecked { freed = wad - burn; } // burn <= wad always
    }

In the free() function, using an outdated rate might allow users to withdraw more collateral, resulting in the collateral value being less than the debt value.

https://github.com/makerdao/dss/blob/fa4f6630afb0624d04a003e920b0d71a00331d98/src/vat.sol#L142C2-L180C6

   // --- CDP Manipulation ---
    function frob(bytes32 i, address u, address v, address w, int dink, int dart) external {
        // system is live
        require(live == 1, "Vat/not-live");

        Urn memory urn = urns[i][u];
        Ilk memory ilk = ilks[i];
        // ilk has been initialised
        require(ilk.rate != 0, "Vat/ilk-not-init");

        urn.ink = _add(urn.ink, dink);
        urn.art = _add(urn.art, dart);
        ilk.Art = _add(ilk.Art, dart);

        int dtab = _mul(ilk.rate, dart);
    @>    uint tab = _mul(ilk.rate, urn.art);
        debt     = _add(debt, dtab);

        // either debt has decreased, or debt ceilings are not exceeded
        require(either(dart <= 0, both(_mul(ilk.Art, ilk.rate) <= ilk.line, debt <= Line)), "Vat/ceiling-exceeded");
        // urn is either less risky than before, or it is safe
    @>    require(either(both(dart <= 0, dink >= 0), tab <= _mul(urn.ink, ilk.spot)), "Vat/not-safe");

        // urn is either more safe, or the owner consents
        require(either(both(dart <= 0, dink >= 0), wish(u, msg.sender)), "Vat/not-allowed-u");
        // collateral src consents
        require(either(dink <= 0, wish(v, msg.sender)), "Vat/not-allowed-v");
        // debt dst consents
        require(either(dart >= 0, wish(w, msg.sender)), "Vat/not-allowed-w");

        // urn has no debt, or a non-dusty amount
        require(either(urn.art == 0, tab >= ilk.dust), "Vat/dust");

        gem[i][v] = _sub(gem[i][v], dink);
        dai[w]    = _add(dai[w],    dtab);

        urns[i][u] = urn;
        ilks[i]    = ilk;
    }

In the frob function, the user’s debt in USD is _mul(ilk.rate, urn.art), and the value of the collateral in USD is _mul(urn.ink, ilk.spot). The user’s debt in USD needs to be less than the value of the collateral in USD. However, due to the use of an outdated rate, the debt value calculation is inaccurate. The actual debt value may already be greater than the collateral value.

Internal pre-conditions

No response

External pre-conditions

jug.drip has not been called for a while, so ilk.rate has not been updated.

Attack Path

Users notice that ilk.rate has not been updated for a considerable amount of time.
Users perform wipe() and free() operations, avoiding the payment of stability fees for this period.

Impact

The protocol suffers a financial loss.

PoC

No response

Mitigation

Add the jug.drip() call in the wipe(), wipeAll(), and free() functions.

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

ZeroTrust - In LockstakeEngine.sol, the functions wipe(), wipeAll(), and free() lack the call to jug.drip(ilk) #84