sherlock-audit / 2024-05-midas-judging

13 stars 6 forks source link

plairfx - `Pausable.sol::pauseAdminRole` will return 0x00 which gives users with no roles the abillity to pause and unpause. #171

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

plairfx

medium

Pausable.sol::pauseAdminRole will return 0x00 which gives users with no roles the abillity to pause and unpause.

Summary

PauseAdminRole function inside Pausable.sol will return 0x00 value, which causes users with no roles to be able to pause and unpause the protocol.

Vulnerability Detail

modifier onlyPauseAdmin is used on the pause and unpause function, which calls for pauseAdminrole

 */
    modifier onlyPauseAdmin() {
        _onlyRole(pauseAdminRole(), msg.sender);
        _;
    }

_onlyRole will check if pauseAdminRole() is equal to msg.sender's role. Which is 0x00 which is equal to the base byte32 return given to every user.

Because pauseAdminRole does not return anything it will return 0x00

    /**
     * @dev virtual function to determine pauseAdmin role
     */
    function pauseAdminRole() public view virtual returns (bytes32);
    //

and when a user has no role, it will return 0x00 which be equal to what the pauseAdminRole will return meaning the user will be able to use pause and unpause in Pausable.sol


    function pause() external onlyPauseAdmin {
        _pause();
    }

    function unpause() external onlyPauseAdmin {
        _unpause();
    }

Impact

Users with no roles will be able to pause and unpause the protocol which only admins should be able to to do.

Code Snippet

https://github.com/sherlock-audit/2024-05-midas/blob/a4a3cc23bb891913ce44665a4cdea9f5c1190f6c/midas-contracts/contracts/access/Pausable.sol#L18-#L21

https://github.com/sherlock-audit/2024-05-midas/blob/a4a3cc23bb891913ce44665a4cdea9f5c1190f6c/midas-contracts/contracts/access/Pausable.sol#L41-L43

Tool used

Manual Review

Recommendation

Make sure pauseadminRole() returns a byte32 that will not equal the base role of every user, but one of the admin/s

sherlock-admin3 commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

WildSniper commented:

this is abstract contract sir and further implementation is is place in main contracts