Flow

Summary

The onlyAdmin modifier may lead to improper error handling if the CallerNotAdmin custom error is not properly defined or utilized. This could result in misleading error messages or unexpected behavior when non-admin addresses attempt to access restricted functions.

Finding Description

The onlyAdmin modifier in the Adminable contract checks if the msg.sender is equal to the admin address. If the check fails, it reverts the transaction using a custom error defined in the Errors library. However, if the CallerNotAdmin error is not properly defined or imported, the revert may fail to compile or produce a misleading error.

This issue breaks the security guarantee that users receive clear and correct feedback on their access rights. If a non-admin user attempts to invoke a function restricted by the onlyAdmin modifier, the lack of proper error handling could lead to confusion about the nature of the failure, potentially allowing unauthorized actions or hiding other vulnerabilities.

Malicious Input Scenario

If a malicious user attempts to call a function protected by onlyAdmin, the expected behavior is to receive a clear revert message indicating that they are not authorized. However, if the error is not defined correctly, they might not receive any error message or could receive a misleading revert reason. This could lead to further exploits or attempts to manipulate the contract, as the user might not understand the access control mechanism.

Vulnerability Details

• Error Handling Issue: The reliance on a custom error (CallerNotAdmin) requires careful definition and importing. Any issues with this error's definition could lead to transaction failures without appropriate feedback.

• Security Implications: The unclear error messages undermine the contract's access control, potentially allowing unauthorized users to probe the system for vulnerabilities without clear feedback.

Impact

The impact of this issue is significant, as it directly affects the usability and security of the contract. Users must receive accurate feedback on their permission levels to maintain trust in the system and avoid potential exploitation. Poor error handling can lead to frustration, increased attack vectors, and difficulties in debugging for developers and users.

Proof of Concept

The current implementation of the onlyAdmin modifier is as follows:

To demonstrate the issue, if the CallerNotAdmin error is not defined in the Errors library or is improperly imported, any calls to functions utilizing this modifier will fail without clear reasoning, resulting in a lack of actionable feedback.

Recommendations

To resolve this issue, ensure that the CallerNotAdmin error is correctly defined and imported in the Errors library. Here is an example of what the error definition looks like:

Additionally, verify the import statement in the Adminable contract:

This will ensure that any calls to functions protected by onlyAdmin provide the correct revert message when access is denied.

File Location

Adminable.sol