Misleading NatSpec comments and potential for the Owner to blacklist itself

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDe.sol#L106-L109 https://github.com/code-423n4/2023-10-ethena/blob/main/contracts/StakedUSDe.sol#L120-L123

Vulnerability details

Both of these issues stem from the same set of functions (addToBlacklist() and removeFromBlacklist()), which handle blacklisting operations in the contract.

Misleading NatSpec comments for roles

In the provided code for the addToBlacklist() and removeFromBlacklist() functions, the NatSpec comments imply that the DEFAULT_ADMIN_ROLE can directly interact with these functions. However, in reality, the DEFAULT_ADMIN_ROLE has the ability to control blacklisting indirectly by manipulating roles using the grantRole() and revokeRole() functions. This misleading documentation also hides the potential risk mentioned in the second issue where the DEFAULT_ADMIN_ROLE might inadvertently blacklist itself.

Potential for DEFAULT_ADMIN_ROLE to blacklist itself

An oversight exists where the DEFAULT_ADMIN_ROLE might blacklist itself, via the grantRole() function. While the addToBlacklist() function does have the notOwner() protection modifier, if it is genuinely intended that the DEFAULT_ADMIN_ROLE should also be able to blacklist but simultaneously be protected from blacklisting itself, additional logic is required. This could be achieved by overriding the _grantRole() function to include checks ensuring the owner does not blacklist themselves.

Impact

• Misleading documentation can cause developers or external integrators to misuse or misinterpret the functions.
• If the DEFAULT_ADMIN_ROLE can blacklist itself, it could render the admin ineffective, potentially stopping essential functions of the contract.

Proof of Concept

1. A user with the DEFAULT_ADMIN_ROLE attempts to use the addToBlacklist() and removeFromBlacklist() functions, but the calls are reverted.
2. A user with the DEFAULT_ADMIN_ROLE employs the grantRole() function and sets themselves as fully restricted.

Tools Used

Manual review

Recommended Mitigation Steps

1. Accurately update the NatSpec comments to reflect the capabilities of the DEFAULT_ADMIN_ROLE.
2. Implement checks to prevent the DEFAULT_ADMIN_ROLE from blacklisting themselves.
3. Override the _grantRole() function and incorporate the necessary checks to make sure only the intended addresses can be assigned specific roles.

Assessed type

Invalid Validation

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #136

[c4-judge]

fatherGoose1 changed the severity to QA (Quality Assurance)

[c4-judge]

fatherGoose1 marked the issue as grade-b