Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BURN_ADMIN_ROLE cannot be set for module roles #142

Open
hats-bug-reporter bot opened this issue Jun 16, 2024 · 1 comment
Open

BURN_ADMIN_ROLE cannot be set for module roles #142

hats-bug-reporter bot opened this issue Jun 16, 2024 · 1 comment
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

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
  1. 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);
+    }
@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Jun 16, 2024
@FHieser
Copy link

FHieser commented Jun 26, 2024

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

@marvinkruse marvinkruse added the invalid This doesn't seem right label Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants