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

1 stars 1 forks source link

Mansa11 - System cannot be turned back on after shutdown Period #122

Closed sherlock-admin3 closed 1 month ago

sherlock-admin3 commented 1 month ago

Mansa11

High

System cannot be turned back on after shutdown Period

Description

In Splitter.sol, There is a variable live which is set to 1 upon deployment and to 0 when the contract is caged during global shutdown. The shutdown period is meant to be activated whenever there are discrepancies in the protocol. However, the current implementation does not make provision for activating the system back on after the issue might have been mitigated

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/dss-flappers/src/Splitter.sol#L122-L126

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/dss-flappers/src/Splitter.sol#L98-L120

Impact

Contract is stuck and cannot be made live anymore due to no provision for the implementation. Furthermore, the main core functionality of the Splitter contract which is Kick will no longer be callable, as it can only be called when live == 1

    function kick(uint256 tot, uint256) external auth returns (uint256) {
   @->  require(live == 1, "Splitter/not-live"); //@audit

        require(block.timestamp >= zzz + hop, "Splitter/kicked-too-soon");
        zzz = block.timestamp;

        vat.move(msg.sender, address(this), tot);

        uint256 lot = tot * burn / RAD;
        if (lot > 0) {
            DaiJoinLike(daiJoin).exit(address(flapper), lot);
            flapper.exec(lot);
        }

        uint256 pay = (tot / RAY - lot);
        if (pay > 0) {
            DaiJoinLike(daiJoin).exit(address(farm), pay);
            farm.notifyRewardAmount(pay);
        }

        emit Kick(tot, lot, pay);
        return 0;
    }

POC

    function cage(uint256) external auth { //@audit no function to uncage and reset live back to 1. will no longer be able to kick
        live = 0;
        emit Cage(0);
    }

Recommendation

Implement a function that allows resetting the live back to initial state after the system must have cooled down.

sunbreak1211 commented 1 month ago

Executing global shutdown (which btw is out of scope) will leave the whole system in a status of no recovery (e.g. the Vat, the most important contract). Redeploying is necessary. So this issue is wrong. Even if you could argue that the cage could be called at local level for the Splitter it is still a design decision. It is not a bug, much less a High severity one.