zhoo - LockstakeEngine.wipe did not update the rate

[sherlock-admin3]

zhoo

Medium

LockstakeEngine.wipe did not update the rate

Summary

LockstakeEngine.wipe did not update the rate

Root Cause

LockstakeEngine.draw uses the return value of jug.drip as rate, and the rate is the updated rate. However, wipe uses the vate.ilks function to obtain the rate, which is not updated at this time.

jug.drip function: https://github.com/makerdao/dss/blob/fa4f6630afb0624d04a003e920b0d71a00331d98/src/jug.sol#L122

    function drip(bytes32 ilk) external returns (uint rate) {
        require(now >= ilks[ilk].rho, "Jug/invalid-now");
        (, uint prev) = vat.ilks(ilk);
@>    rate = _rmul(_rpow(_add(base, ilks[ilk].duty), now - ilks[ilk].rho, ONE), prev);
        vat.fold(ilk, vow, _diff(rate, prev));
        ilks[ilk].rho = now;
    }

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

    function draw(address urn, address to, uint256 wad) external urnAuth(urn) {
@>   uint256 rate = jug.drip(ilk);
        uint256 dart = _divup(wad * RAY, rate);
        require(dart <= uint256(type(int256).max), "LockstakeEngine/overflow");
        vat.frob(ilk, urn, address(0), address(this), 0, int256(dart));
        nstJoin.exit(to, wad);
        emit Draw(urn, to, wad);
    }

    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);
    }

See the wipe implementation in another older version:

https://github.com/makerdao/dss-allocator/blob/6304bfd3f567630636244cb2ca3b58dd415592fa/src/AllocatorVault.sol#L142

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

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. The user runs LockstakeEngine.draw
2. rate has been updated after a period of time, but no other user has performed draw, and rate has not been updated.
3. The user executed LockstakeEngine.wipe using the old rate

Impact

The old rate was used, resulting in the loss of funds.

PoC

No response

Mitigation

    function wipe(uint256 wad) external auth {
-      (, uint256 rate,,,) = vat.ilks(ilk);
+      uint256 rate = jug.drip(ilk);
    }

Duplicate of #66

[sunbreak1211]

This is clearly specified in the scope...

"wipeAll and wipe do not drip because it is actually not convenient for the user to do a drip call on wipping. Then, if we force the drip, we are incentivizing users to repay directly to the vat (which is possible) instead of using the engine for that. We are mimicking the old proxy actions behaviour, where we drip for drawing, as otherwise the user can lose money, but not forcing the drip on wiping so users actually use this function."