kevinkien - Owner possible to set `Owner` and `Authority` to the zero address.

[sherlock-admin4]

kevinkien

High

Owner possible to set Owner and Authority to the zero address.

Summary

The SplitterMom contract has two functions, setOwner and setAuthority, that allow changing the contract owner's address (owner) and the authority address (authority). However, neither function includes checks to prevent these addresses from being set to the zero address (0x0).

Vulnerability Detail

• setOwner(address _owner): This function allows the current owner to change the contract's owner. However, it does not verify if the new _owner address is the zero address.
• setAuthority(address _authority): This function allows the current owner to change the authority address. Similar to setOwner, it does not check if the new _authority address is the zero address.

Impact

Setting owner or authority to the zero address can lead to the following consequences:

• Loss of Control: If owner is set to the zero address, it will be impossible to change the owner or authority again, rendering the contract unmanageable.
• Increased Security Risk: If authority is set to the zero address, the authority-based access control mechanism will be disabled, making the contract more vulnerable to attacks.

Code Snippet

https://github.com/sherlock-audit/2024-06-makerdao-endgame/blob/main/dss-flappers/src/SplitterMom.sol#L68-L76

    function setOwner(address _owner) external onlyOwner {
        owner = _owner;
        emit SetOwner(_owner);
    }

    function setAuthority(address _authority) external onlyOwner {
        authority = _authority;
        emit SetAuthority(_authority);
    }

Tool used

Manual Review

Recommendation

require checks should be added to both functions to ensure that the provided new addresses are not the zero address:

function setOwner(address _owner) external onlyOwner {
+    require(_owner != address(0), "SplitterMom/new-owner-cannot-be-zero");
    owner = _owner;
    emit SetOwner(_owner);
}

function setAuthority(address _authority) external onlyOwner {
+    require(_authority != address(0), "SplitterMom/new-authority-cannot-be-zero");
    authority = _authority;
    emit SetAuthority(_authority);
}

[sunbreak1211]

This is not an issue.