revoked roles can still have access to modules and new roles may not be granted successfully

[hats-bug-reporter[bot]]

Github username: @Audinarey Twitter username: audinarey Submission hash (on-chain): 0x3e2f16039e1880e6e83efdb93abd1b55ad96b80e17faaa56eb46b5630e21d6c4 Severity: medium

Description: Description\ This issue can lead to a scenario where

• revoked roles still have authorised access to modules
• roles are not granted successfully without issues

The AUT_Roles_v1 contract inherits from AccessControlEnumerableUpgradeable contract. When roles are granted or revoked either singly or in batches, the AccessControlEnumerableUpgradeable::_grantRole(...) and AccessControlEnumerableUpgradeable::_revokeRoleRole(...) functions are called internally to complete the process.

However, these functions return an unchecked boolean granted and revoked for each perculiar case that signifies whether or not the roles were successfully granted or revoked.

File: AccessControlEnumerable.sol
49:     /**
50:      * @dev Overload {AccessControl-_grantRole} to track enumerable memberships
51:      */
52:     function _grantRole(bytes32 role, address account) internal virtual override returns (bool) {
53:         bool granted = super._grantRole(role, account);
54:         if (granted) {
55:             _roleMembers[role].add(account);
56:         }
57:         return granted;
58:     }
59: 
60:     /**
61:      * @dev Overload {AccessControl-_revokeRole} to track enumerable memberships
62:      */
63:     function _revokeRole(bytes32 role, address account) internal virtual override returns (bool) {
64:         bool revoked = super._revokeRole(role, account);
65:         if (revoked) {
66:             _roleMembers[role].remove(account);
67:         }
68:         return revoked;
69:     }
solidity

These checks are not implemented in the AUT_Roles_v1 contract and as such it is possible that a role/roles is/are grant or revoked from a module without success and the call will not revert.

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

   Modify the

   • AUT_Roles_v1::grantRoleFromModule(...)
   • AUT_Roles_v1::grantRoleFromModuleBatched(...)
   • AUT_Roles_v1::revokeRoleFromModule(...)
   • AUT_Roles_v1::revokeRoleFromModuleBatched(...) functions respectively as shown below

File: AUT_Roles_v1.sol
183: 
184:     /// @inheritdoc IAuthorizer_v1
185:     function grantRoleFromModule(bytes32 role, address target)
186:         external
187:         onlyModule(_msgSender())
188:     {
189:         bytes32 roleId = generateRoleId(_msgSender(), role);

190:   -         _grantRole(roleId, targets[i]);
190.   +        require(_grantRole(roleId, targets[i]), "Error granting role");

191:     }
192: 
193:     /// @inheritdoc IAuthorizer_v1
194:     function grantRoleFromModuleBatched(
195:         bytes32 role,
196:         address[] calldata targets
197:     ) external onlyModule(_msgSender()) {
198:         bytes32 roleId = generateRoleId(_msgSender(), role);
199:         for (uint i = 0; i < targets.length; i++) {

200:   -         _grantRole(roleId, targets[i]);
200.   +        require(_grantRole(roleId, targets[i]), "Error granting role");

201:         }
202:     }
203: 
204:     /// @inheritdoc IAuthorizer_v1
205:     function revokeRoleFromModule(bytes32 role, address target)
206:         external
207:         onlyModule(_msgSender())
208:     {
209:         bytes32 roleId = generateRoleId(_msgSender(), role);

210:   -     _revokeRole(roleId, target);
210.   +        require(_revokeRole(roleId, targets[i]), "Error revoking role");

211:     }
212: 
213:     /// @inheritdoc IAuthorizer_v1
214:     function revokeRoleFromModuleBatched(
215:         bytes32 role,
216:         address[] calldata targets
217:     ) external onlyModule(_msgSender()) {
218:         bytes32 roleId = generateRoleId(_msgSender(), role);
219:         for (uint i = 0; i < targets.length; i++) {

220:    -        _revokeRole(roleId, targets[i]);
220:    +        require(_revokeRole(roleId, targets[i]), "Error revoking role");

221:         }
222:     }
223: 

[FHieser]

Please provide a POC