zhoo - LockstakeClipper.yank did not process the burned lsmkr token

[sherlock-admin4]

zhoo

Medium

LockstakeClipper.yank did not process the burned lsmkr token

Summary

LockstakeClipper.yank did not process the burned lsmkr token, resulting in loss of funds.

Root Cause

When the user is liquidated, lsmkr.burn is executed in LockstakeEngine.onKick, However, when canceling the auction using LockstakeClipper.yank, no processing is done on the lsmkr token.

LockstakeClipper.kick -> LockstakeEngine.onKick -> lsmkr.burn

    function onKick(address urn, uint256 wad) external auth {
        // Urn confiscation happens in Dog contract where ilk vat.gem is sent to the LockstakeClipper
        (uint256 ink,) = vat.urns(ilk, urn);
        uint256 inkBeforeKick = ink + wad;
        _selectVoteDelegate(urn, inkBeforeKick, urnVoteDelegates[urn], address(0));
        _selectFarm(urn, inkBeforeKick, urnFarms[urn], address(0), 0);
@>    lsmkr.burn(urn, wad);
        urnAuctions[urn]++;
        emit OnKick(urn, wad);
    }

lsmkr token is not processed when yank:

    function yank(uint256 id) external auth lock {
        require(sales[id].usr != address(0), "LockstakeClipper/not-running-auction");
        dog.digs(ilk, sales[id].tab);
        uint256 lot = sales[id].lot;
        vat.flux(ilk, address(this), msg.sender, lot);
        engine.onRemove(sales[id].usr, 0, 0);
        _remove(id);
        emit Yank(id);
    }

    function onRemove(address urn, uint256 sold, uint256 left) external auth {
        uint256 burn;
        uint256 refund;
        if (left > 0) {
            burn = _min(sold * fee / (WAD - fee), left);
            mkr.burn(address(this), burn);
            unchecked { refund = left - burn; }
            if (refund > 0) {
                // The following is ensured by the dog and clip but we still prefer to be explicit
                require(refund <= uint256(type(int256).max), "LockstakeEngine/overflow");
                vat.slip(ilk, urn, int256(refund)); 
                vat.frob(ilk, urn, urn, address(0), int256(refund), 0);
                lsmkr.mint(urn, refund);
            }
        }
        urnAuctions[urn]--;
        emit OnRemove(urn, sold, burn, refund);
    }

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

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

Internal pre-conditions

1. The user is liquidated and enters the auction state.
2. The auction is canceled

External pre-conditions

No response

Attack Path

1. The user is liquidated and enters the auction state.
2. The administrator calls LockstakeClipper.yank to cancel the auction.
3. The lkmkr token burned during LockstakeEngine.onKick is not processed, resulting in a loss of funds.

Impact

Cause the loss of funds.

PoC

No response

Mitigation

The yank function sends the burned lsmkr token to the msg.sender

Duplicate of #83

[telome]

From the contest readme: "Using yank() in the lockstake clipper is assumed to only happen as part of a shutdown procedure. Since this is out of scope, it is assumed not to happen."