hats-finance / Velvet-Capital-0x0bb0c08fd9eeaf190064f4c66f11d18182961f77

Core smart contracts of Velvet Capital
Other
0 stars 1 forks source link

roles can be renounced by role address holders #18

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xf2a758e2ec8944da3d20a5f5572b98f0017782600a0e45907040482f6e69604a Severity: low

Description: Description\

AccessController.sol contract is used to manage roles and permissions within the Portfolio platform. Various roles are granted via setUpRoles() function which can only be called by admin.

Roles mentioned [here] can simply renounce their role as AccessController has inherited openzeppelin's AccessControl.sol contract which allows to call renounceRole() which is implemented as:

    function renounceRole(bytes32 role, address callerConfirmation) public virtual {
        if (callerConfirmation != _msgSender()) {
            revert AccessControlBadConfirmation();
        }

        _revokeRole(role, callerConfirmation);
    }

Therefore, roles assigned to addresses can break the whole functionality of contracts as these roles are assigned with some purposese across Velvet protocol.

Recommendations\ Restrict roles addresses from renouncing the roles.

-    function renounceRole(bytes32 role, address callerConfirmation) public virtual {
+    function renounceRole(bytes32 role, address callerConfirmation) public virtual override {
+        revert("can not renounce role");
     }    
langnavina97 commented 1 week ago

We have a super admin role who can grant the roles again if necessary. Additionally, the functionality allowing roles to be renounced is by design, following the OpenZeppelin AccessControl implementation.

0xRizwan commented 1 week ago

@langnavina97 Yes, super admin can grant role again and that will be additional work for admin. renounce of roles should be restricted so the protocols functionality with associated roles should not be hampered even for short period of time.