wipe function will use stale rate when ilk in the engine is not updated through Jug.drip()

Summary

Missing drip call to the jug, will result of users paying their debt using stale rate. https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/lockstake/src/LockstakeEngine.sol#L391, which is cheaper than new rate. which means loss for the protocol.

Root Cause

missing drip call to the jug when calling wipe function in the LockstakeEngine.sol

Internal pre-conditions

anyone can call wipe

External pre-conditions

ilk in the lockstakeengine not being called for some period of times

Attack Path

get a loan by calling draw()
call wipe()

Impact

pay the debt using stale rate, which doesnt reflect the actual rate of the ilk

PoC

modified the test for testDrawWipe()

    function testDrawWipe() public {
        deal(address(mkr), address(this), 100_000 * 10**18, true);
        address urn = engine.open(0);
        mkr.approve(address(engine), 100_000 * 10**18);
        engine.lock(urn, 100_000 * 10**18, 5);
        assertEq(_art(ilk, urn), 0);
        vm.expectEmit(true, true, true, true);
        emit Draw(urn, address(this), 50 * 10**18);
        engine.draw(urn, address(this), 50 * 10**18);
        assertEq(_art(ilk, urn), 50 * 10**18);
        assertEq(_rate(ilk), 10**27);
        assertEq(nst.balanceOf(address(this)), 50 * 10**18);
        vm.warp(block.timestamp + 1);
        vm.expectEmit(true, true, true, true);
        emit Draw(urn, address(this), 50 * 10**18);
        engine.draw(urn, address(this), 50 * 10**18);
        uint256 art = _art(ilk, urn);
        uint256 expectedArt = 50 * 10**18 + _divup(50 * 10**18 * 100000000, 100000001);
        assertEq(art, expectedArt);
        uint256 rate = _rate(ilk);
        assertEq(rate, 100000001 * 10**27 / 100000000);
        assertEq(nst.balanceOf(address(this)), 100 * 10**18);
        assertGt(art * rate, 100.0000005 * 10**45);
        assertLt(art * rate, 100.0000006 * 10**45);
        vm.expectRevert("Nst/insufficient-balance");
        engine.wipe(urn, 100.0000006 * 10**18);
        address anyone = address(1221121);
        deal(address(nst), anyone, 100.0000006 * 10**18, true);
        assertEq(nst.balanceOf(anyone), 100.0000006 * 10**18);
        vm.prank(anyone); nst.approve(address(engine), 100.0000006 * 10**18);
        vm.expectRevert();
        vm.prank(anyone); engine.wipe(urn, 100.0000006 * 10**18); // It will try to wipe more art than existing, then reverts
        vm.expectEmit(true, true, true, true);
        emit Wipe(urn, 100.0000005 * 10**18);

        console.log("non revert wipe");
        vm.prank(anyone);
        vm.warp(block.timestamp + 3 days); //move forward 3 days
        IJug(0x19c0976f590D67707E62397C87829d896Dc0f1F1).drip(ilk); //comment this to make the test run like the original one
        engine.wipe(urn, 100.0000005 * 10**18);
        console.log(nst.balanceOf(anyone));
        vm.warp(block.timestamp - 3 days); // return to the original block.timestamp
        //without drip
        // non revert wipe
        // 100000000000

        assertEq(nst.balanceOf(anyone), 0.0000001 * 10**18);
        assertEq(_art(ilk, urn), 1); // Dust which is impossible to wipe via this regular function
        emit Wipe(urn, _divup(rate, RAY));
        vm.prank(anyone); assertEq(engine.wipeAll(urn), _divup(rate, RAY));
        assertEq(_art(ilk, urn), 0);
        assertEq(nst.balanceOf(anyone), 0.0000001 * 10**18 - _divup(rate, RAY));
        address other = address(123);
        assertEq(nst.balanceOf(other), 0);
        emit Draw(urn, other, 50 * 10**18);
        engine.draw(urn, other, 50 * 10**18);
        assertEq(nst.balanceOf(other), 50 * 10**18);
        // Check overflows
        stdstore.target(address(dss.vat)).sig("ilks(bytes32)").with_key(ilk).depth(1).checked_write(1);
        assertEq(_rate(ilk), 1);
        vm.expectRevert("LockstakeEngine/overflow");
        engine.draw(urn, address(this), uint256(type(int256).max) / RAY + 1);
        stdstore.target(address(dss.vat)).sig("dai(address)").with_key(address(nstJoin)).depth(0).checked_write(uint256(type(int256).max) + RAY);
        deal(address(nst), address(this), uint256(type(int256).max) / RAY + 1, true);
        nst.approve(address(engine), uint256(type(int256).max) / RAY + 1);
        vm.expectRevert("LockstakeEngine/overflow");
        engine.wipe(urn, uint256(type(int256).max) / RAY + 1);
        stdstore.target(address(dss.vat)).sig("urns(bytes32,address)").with_key(ilk).with_key(urn).depth(1).checked_write(uint256(type(int256).max) + 1);
        assertEq(_art(ilk, urn), uint256(type(int256).max) + 1);
        vm.expectRevert("LockstakeEngine/overflow");
        engine.wipeAll(urn);
    }

Mitigation

add jug.drip(ilk) like in the draw(), to use fresh rate when paying the debt.

Duplicate of #66

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

Random_dude - wipe function will use stale rate when ilk in the engine is not updated through Jug.drip() #125