hats-finance / Blast-Futures-Exchange-0x97895c329b950755566ddcdad3395caaea395074

0 stars 0 forks source link

Owner can be admin even if removed from the role through BfxVault#makeOwnerAdmin #44

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

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

Github username: @devblixt Twitter username: -- Submission hash (on-chain): 0x13bc2d5b3933245996c6f633183899198612e60f7c00afd1a4ffa1429d6ba29c Severity: medium

Description: Description\ BfxVault provides a function makeOwnerAdmin to make the owner of the contract an admin. It also has a removeRole function to remove the role of an EOA. However, even if the owner is removed from the admin role, they can be added back with the makeOwnerAdmin function.

Attack Scenario\

  1. The admin removes owner as an admin because of some issue, such as if owner's address is compromised.
  2. The owner makes themselves admin again.

Attachments

  1. Proof of Concept (PoC) File This issue is trivial and does not need a PoC file.
chainNue commented 9 months ago

By design, owner is immutable, (as there is also no transfer ownership function), so if owner address is compromised, there would be much more critical issue related with onlyOwner modifier functions. IMO, the makeOwnerAdmin function is a failsafe function called by owner, when a certain admin became rough by removing owner as admin, #23

alex-sumner commented 9 months ago

This is not a bug. An attacker would need to know the private keys of the owner account in order to call makeOwnerAdmin.