Rhinestone Core Modules Audit Summary

Rhinestone Core Modules are a set of smart account modules to extend smart account capabilities. The Core Modules bundle contains the following modules developed by Rhinestone:

• AutoSavings: Automatically save a portion of received tokens into a vault
• ColdStorageHook: Secure your account by transforming it into cold storage for your assets
• ColdStorageFlashloan: Enable using the utility of your assets in cold storage through flashloans
• DeadmanSwitch: Secure your account by setting up a deadman switch
• HookMultiPlexer: Use multiple hooks based on granular conditions
• MultiFactor: Multiplex different validators to make your account more secure
• OwnableExecutor: Control your account from another account
• OwnableValidator: Own your account using an EOA or a set of EOAs
• RegistryHook: Use the Rhinestone Registry to ensure you only install secure modules
• ScheduledOrders: Automate swaps on a schedule
• ScheduledTransfers: Automate transfers on a schedule
• SocialRecovery: Recover your account using a set of trusted friends

Rhinestone engaged Ackee Blockchain to perform a security review of the Rhinestone Core Modules for a total of 21 engineering days in a period between April 29 and May 24, 2024.

METHODOLOGY

We began our review using static analysis tools, including Wake in companion with Tools for Solidity VS Code extension. We then took a deep dive into the logic of the contracts. For testing and fuzzing, we have involved Wake testing framework.

During the review, we paid special attention to:

• checking the logic of examples according to specifications,
• checking the assets cannot be locked or lost,
• validating ERC-3156 flashloans implementation,
• checking ERC-4337 restrictions are followed,
• detecting possible reentrancies in the code,
• ensuring the arithmetic of the system is correct,
• ensuring access controls are not too weak or too strict, 
• looking for common issues such as data validation.

SCOPE

The audit has been performed on the commit 013a123 and the exact scope was the following files:

• Core Modules, excluding external dependencies,
• SentinelList library (f3f84d6),
• CheckNSignatures library (53617ec).

FINDINGS

Here we present our findings.

Critical severity

No critical severity issues were found. 

High severity

H1: Missing threshold checks

H2: Removing from a wrong array of sigs in removeSigHook

H3: OwnableExecutor locked Ether

H4: ERC-4337 restricted storage access

H5: Nominee have limited access

H6: Externally increasable borrower’s nonce

H7: ERC-3156 flashloans implementation

Medium severity

M1: Missing sqrtPriceLimitX96 check

M2: Removing different address

M3: Missing module type condition

Low severity

L1: HookMultiPlexer with no hooks

L2: flashLoan front-run

L3: Unsafe ERC-20 calls

L4: Missing initialized check in SentinelList

L5: Missing deletion of execution element

L6: Excluding list element

Warning severity

W1: MultiFactor duplicate validators

W2: Missing clearTrustedForwarder call

W3: SchedulingBase executions count validation

W4: Missing zero address check

W5: Missing array length validation

W6: Missing value check in ERC-20 transfers

W7: TODOs in module HookMultiPlexer

Informational severity

I1: AutoSavings percentage precision

I2: Unused code

I3: Unused variable

I4: Internal functions missing prefix

I5: Missing events

I6: Typos and incorrect documentation

I7: Redundant assignments in SentinelList

I8: Missing function restriction

I9: Proposal for refactoring HookMultiPlexer

CONCLUSION

Our review resulted in 32 findings ranging from Info to High severity.

The most severe high issues point to various problems in the codebase such as missing threshold checks (H1), removing a hook from a different list (H2), locked Ether (H3), ERC-4337 restricted storage access (H4), updating waitPeriod for the nominee (H5), externally increasable borrower’s nonce (H6) and many violations in ERC-3156 flashloans implementation (H7). Since the codebase contains major problems, we do not recommend deploying and using the contracts until all the severe issues are resolved. The code is mostly well documented, but the code quality is not as polished as the reference examples should be.

 

Ackee Blockchain recommends Rhinestone to:

• add the threshold protection when removing validators/owners,
• avoid locking assets in the contract,
• prevent interacting with restricted storage slots according to ERC-4337 rules,
• fix lastAccess timestamp resetting for a nominee in DeadmanSwitch contract,
• fix bypassing whitelist and nonce increase in ColdStorageFlashloan contract,
• strictly follow the ERC-3156 specification,
• add a check for slippage protection in ScheduledOrders contract,
• fix the SentinelList.pop function parameters order in ColdStorageFlashLoan.removeAddress ,
• fix module types condition in ColdStorageHook function,
• address all other reported issues,
• perform a complete internal code review to ensure better code quality,
• complete the missing documentation.

 

Ackee Blockchain’s full Rhinestone audit report with a more detailed description of all findings and recommendations can be found here.

 

We were delighted to audit Rhinestone and look forward to working with them again.