chaduke - PausableImpl() lacks some sanity checks

[sherlock-admin]

chaduke

medium

PausableImpl() lacks some sanity checks

Summary

PausableImpl lacks some sanity checks, including 1) zero address check for owner, 2) checking state of $paused for function setPaused().

Vulnerability Detail

PausableImpl is an abstract contract that allows the owner to pause/unpause a contract and add the modifier pausable to a function so that the function can be called only when the contract is not paused.

https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-utils/src/PausableImpl.sol#L9-L62

However, there are two issues for the current implementation:

1. The __initPausable() lacks the zero address check for owner_. If zero address is entered for owner_ by mistake or on purpose, the ownership will be lost forever. Costing redeployment might be necessary.

function __initPausable(address owner_, bool paused_) internal virtual {
        OwnableImpl.__initOwnable(owner_);
        $paused = paused_;
    }

2. The setPaused() performs a blind overwritting to the state of $paused before checking the previous state, and emit unnecessary SetPaused(paused_) event when the new state of $paused is the same as the old one.

 function setPaused(bool paused_) public virtual onlyOwner {
        $paused = paused_;
        emit SetPaused(paused_);
    }

A check is needed to avoid making unnecessary state change and redundant event emitting.

Impact

If zero address is entered for owner, the contract will lost ownership forever and a redeployment is necessary. The unnecessary SetPaused(paused_) event might unnecessarily trigger some applications that depend on the state of $paused such as trading strategies.

Code Snippet

See above

Tool used

Remix

Manual Review

Recommendation

Add a zero address check for the input owner.

setPaused() is revised as follows:

 function setPaused(bool paused_) public virtual onlyOwner {
+     if($pause == paused_) pausedStateNoChange();

        $paused = paused_;
        emit SetPaused(paused_);
    }