removeRole function is public instead of internal

[hats-bug-reporter[bot]]

Github username: -- Twitter username: 97Sabit Submission hash (on-chain): 0xe26ecbf356b1bf76581d1a277af60de253656cc8c68c10cdf390178d8e6fd213 Severity: medium

Description: Description

Similar to the addRole function, the removeRole function in BfxVault is currently marked as public. This allows any authorized caller to directly call removeRole instead of calling the specific role revocation functions like removeAdmin, removeTrader, etc.

For instance, an admin could mistakenly call:

removeRole(user, ADMIN_ROLE)

When they meant to call:

removeTreasurer(user)

This would incorrectly revoke the admin role from the user instead of revoking the treasurer role.

The removeRole function should be marked internal since it is only intended to be called by the other role revocation functions. This would prevent any authorized caller from mistakenly revoking the wrong role by calling removeRole directly.

Recommendation:
Change the removeRole function visibility from public to internal.

This will prevent improper role revocations from authorized callers accidentally calling removeRole instead of the specific role revocation functions.

1. Proof of Concept (PoC) File

https://github.com/hats-finance/Blast-Futures-Exchange-0x97895c329b950755566ddcdad3395caaea395074/blob/d9b402f5124f1470f979ed9a6d010d89988f6dee/foundry/src/BfxVault.sol#L206

[ololade97]

Unfortunately, the same function can be used for other unintended things too. The function is like a double-edged sword.