Audinarey - `_requireAtLeastOperator()` is implemented wrongly breaking core protocol functionality

[sherlock-admin4]

Audinarey

Medium

_requireAtLeastOperator() is implemented wrongly breaking core protocol functionality

Summary

The _requireAtLeastOperator() function is used exclusively to protect most privilege functions in the protocol. It is intended to ensure that only a caller that admin role or at least an operator role can call the functions that is protects.

As shown below, it check to ensure that the caller has both the admin and operator roles.

File: DefaultAccessControl.sol
53:     function _requireAtLeastOperator(address sender) internal view {
54: @>      if (!isAdmin(sender) && !isOperator(sender)) revert Forbidden();
55:     }

Vulnerability Detail

Assume for instance that

• Sam has both OPERATOR and ADMIN_ROLE roles respectively.
• Bob has ADMIN_ROLE role.
• Alice has OPERATOR role.

If

• Alice calls Vault::processWithdrawals(...) it will revert because, although she is an operator but she is not also an admin
• Bob calls Vault::processWithdrawals(...) it will revert because, although he is an admin but he is not also an operator
• Sam calls Vault::processWithdrawals(...) it will not revert because he is both an admin and an operator

So the goal of giving access to an account that has at least an OPERATOR role is defeated

Impact

The implementation breaks core protocol functionality

Code Snippet

https://github.com/sherlock-audit/2024-06-mellow/blob/main/mellow-lrt/src/utils/DefaultAccessControl.sol#L53

Tool used

Manual Review

Recommendation

Modify the _requireAtLeastOperator() function as shown below

File: DefaultAccessControl.sol
53:     function _requireAtLeastOperator(address sender) internal view {

54: -       if (!isAdmin(sender) && !isOperator(sender)) revert Forbidden();
54: +       if (!isAdmin(sender) || !isOperator(sender)) revert Forbidden();

55:     }