addRole function is public instead of internal

[hats-bug-reporter[bot]]

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

Description: Description

The addRole function in BfxVault is currently marked as public. This allows any authorized caller to directly call addRole instead of calling the specific role assignment functions like addAdmin, addTrader, etc.

For example, an admin could accidentally call:

addRole(user, TREASURER_ROLE)

When they meant to call:

addTrader(user)

This would incorrectly give the user the treasurer role instead of the trader role.

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

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

This will prevent improper role assignments from authorized callers accidentally calling addRole instead of the specific role assignment functions.

1. Proof of Concept (PoC) File

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

[alex-sumner]

This is intentional, addRole can be used to add other roles than trader, treasurer and admin. Any integer role id can be used, and may be queried by the off chain code.

[ololade97]

Yes, it can be used to add other roles. Likewise, it can be used to overwrite pre-specified roles.