create2 collision can break urn's ilk balance assumption causing unliquidateable/withdrawable positions

Summary

User's can make lockstake positions to be non-withdrawable/liquidateable by making a tiny donation of gem. This is possible by obtaining a create2 collision for an urn

Vulnerability Detail

Urn's are implemented with an assumption that they have as much lsMkr and mkr as their ink balance in the vat

for ex:

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

But this assumption can be broken if a user manages to donate gems to an urn. Although under normal conditions this is prevented by only the lockstakeengine having access to the urn's gems, a user can make use of create2 collision to preapprove access to another address. create2 collision is achieveable here since the urn's are created with a user controllable salt reducing the required combinations for brute force to the order of ~2^80. This create2 collision feasibility is discussed in this past issue here

gh link

    function open(uint256 index) external returns (address urn) {
        require(index == usrAmts[msg.sender]++, "LockstakeEngine/wrong-urn-index");
        uint256 salt = uint256(keccak256(abi.encode(msg.sender, index)));
        bytes memory initCode = _initCode();

After finding a collision, an attacker can deploy a contract to the urn address, call vat.hope to approve another user to spend its gems/ink and then selfdestruct itself so that the actual urn can be deployed to the same address. Then a user can move the gems freely and perform donations to their own positions to avoid liquidation and also to other user's positions to cause dos for them (ie. unable to change their votes/farms unless they exit fully, paying the fees etc with votes being sensitive)

Similarly a create2 collision found for votedelegate contract will allow a user to lock the funds of all the delegators by setting an approval for the IOU token early and then moving it later to another address causing the withdraw function of chief to revert since that much amount of IOU tokens are not present to burn

    function free(uint wad)
        public
        note
    {
        require(block.number > last[msg.sender]);
        deposits[msg.sender] = sub(deposits[msg.sender], wad);
        subWeight(wad, votes[msg.sender]);
        IOU.burn(msg.sender, wad);

Impact

A user ready to work on finding a collision can avoid future liquidations and also DOS other user's in withdrawing funds

Code Snippet

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

Tool used

Manual Review

Recommendation

Keep an internal accounting for the locked assets/avoid create2 for urn creation

Duplicate of #64

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

hash - create2 collision can break urn's ilk balance assumption causing unliquidateable/withdrawable positions #109