hats-finance / Inverter-Network-0xe47e52c4fea05e555920f1dcdcc6fb8eca103eeb

Fork of the Inverter Smart Contracts Repository
GNU Lesser General Public License v3.0
0 stars 3 forks source link

`BURN_ADMIN_ROLE` cannot be set for module roles #142

Open hats-bug-reporter[bot] opened 2 months ago

hats-bug-reporter[bot] commented 2 months ago

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

Description: Description\ The AUT_Roles_v1::burnAdminFromModuleRole(...) is implemented such that a module role can be set to BURN_ADMIN_ROLE from the module and can be called by only a module. However, the module does not implement any means to call the function ans as such the module roles cannot be set to BURN_ADMIN_ROLE


File: AUT_Roles_v1.sol

233:     function burnAdminFromModuleRole(bytes32 role)
234:         external
235:         onlyModule(_msgSender())
236:     {
237:         bytes32 roleId = generateRoleId(_msgSender(), role);
238:         _setRoleAdmin(roleId, BURN_ADMIN_ROLE);
239:     }

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

    Implement a method for the modules to call the burnAdminFromModuleRole(...) function from within the module as shown below


File: Module_v1.sol

+  function burnModuleAdmin(bytes32 role)
+      external
+      onlyModuleRoleAdmin(role)
+    {
+        __Module_orchestrator.authorizer().burnAdminFromModuleRole(role);
+    }
FHieser commented 2 months ago

Yes and no Apperently we didnt have any need up until now to burnAnAdmin from a role in the first place The internal functions are helper functions, that make it easier for Moduledevs to interact with internal contracts, but they are technically not needed here.' I would rate this as invalid, because the burnModuleAdmin is technically not needed, as the burnAdminFromModuleRole is exposed via external already