sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

martin - Misleading naming could result in not functional contracts and wrong assumptions #110

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

martin

medium

Misleading naming could result in not functional contracts and wrong assumptions

Summary

Likelihood Medium since it requires wrong assumptions in the front-end implementation or users directly interacting with the protocol

Impact Medium because the UX can become terrible

Vulnerability Detail

Wrong naming can give wrong assumptions and can lead to not functional contract if the front-end does not predict this strange behavior.

Impact

The event creates wrong assumptions that the contract is paused when it is not (only the argument is different). The function setPaused on the other side suppose to only pause but actually can unpause as well.

Code Snippet

51: function setPaused(bool paused_) public virtual onlyOwner {
52:     $paused = paused_;
53:     emit SetPaused(paused_);
54:}

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

Tool used

Manual Review

Recommendation

Consider renaming the setPaused function and SetPaused event. Better alternatives are setPausingState for the function name and adding 2 separate events - Paused and UnPaused.