Udsen - `AvaildBridge.initialize` FUNCTION DOES NOT CHECK THE RETURN BOOLEAN VALUE OF `_grantRole(PAUSER_ROLE, pauser)` CALL FOR SUCCESS THUS PUTTING THE ENTIRE BRIDGE IN DANGER IN THE EVENT OF AN EMERGENCY

[sherlock-admin]

Udsen

high

AvaildBridge.initialize FUNCTION DOES NOT CHECK THE RETURN BOOLEAN VALUE OF _grantRole(PAUSER_ROLE, pauser) CALL FOR SUCCESS THUS PUTTING THE ENTIRE BRIDGE IN DANGER IN THE EVENT OF AN EMERGENCY

Summary

AvaildBridge.initialize function does not check the return boolean value for success while calling the _grantRole internal function to set the PAUSER_ROLE to the pauser address.

Vulnerability Detail

The AvaildBridge.initialize function is used to initialize the state of the AvaildBridge contract. This function calls the AccessControlDefaultAdminRulesUpgradeable._grantRole inherited internal function to grant the PAUSER_ROLE to the pauser address as shown below:

    _grantRole(PAUSER_ROLE, pauser);

The _grantRole function returns a bool value indicating the success of the operation as shown below in its function description:

function _grantRole(bytes32 role, address account) internal virtual override returns (bool) {

The issue is the AvaildBridge.initialize function does not check this return bool value.

Impact

As a result even though the _grantRole transaction fails the initialzie function will not detect the failure and would execute the initialize transaction as succesful. So the governance will expect the PAUSER_ROLE to have been set correctly where as it has not.

As a result the AvailBridge will not able to be paused at an emergency since the pauser address provided during the initialization has not been set the PAUSER_ROLE thus putting the entire bridge in danger. Furthermore since the PAUSER_ROLE is only set in the AvailBridge.initialize function and it can be only called once, if not correctly set at the first time there is no logic in the AvaliBridge contract to reconfigure the PAUSER_ROLE to the pauser address or any other address.

Code Snippet

        _grantRole(PAUSER_ROLE, pauser);

https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L101

Tool used

VSCode and Manual Review

Recommendation

Hence it is recommended to check the return bool value of the _grantRole(PAUSER_ROLE, pauser) function call inside the AvaildBridge.initialize function and revert the transaction if the bool value is false. And the initialization should only proceed as a succesful transaction if the bool value is true.

[sherlock-admin]

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

takarez commented:

invalid because { admin func}