Rhinestone Core Modules Security Audit Summary Report

Rhinestone Core Modules offer a collection of smart account functionalities designed to enhance the capabilities of smart accounts. The Core Modules package includes the following modules created by Rhinestone:

AutoSavings: Automatically allocate a portion of incoming tokens to a vault
ColdStorageHook: Fortify your account by converting it into cold storage for your assets
ColdStorageFlashloan: Utilize your assets stored in cold storage via flashloans
DeadmanSwitch: Protect your account by implementing a deadman switch
HookMultiPlexer: Leverage multiple hooks based on specific conditions
MultiFactor: Employ various validators to enhance the security of your account
OwnableExecutor: Manage your account from an alternate account
OwnableValidator: Gain ownership of your account via an EOA or a set of EOAs
RegistryHook: Utilize the Rhinestone Registry to guarantee the installation of only secure modules
ScheduledOrders: Automate token swaps at designated intervals
ScheduledTransfers: Automate token transfers at specified times
SocialRecovery: Retrieve your account with the help of trusted friends

Rhinestone enlisted Ackee Blockchain to carry out a security audit of the Rhinestone Core Modules for a total of 21 engineering days from April 29 to May 24, 2024.

METHODOLOGY

Our review commenced with static analysis tools, integrating Wake alongside the Tools for Solidity VS Code extension. We then performed an in-depth examination of the contract logic. For testing and fuzzing, we used the Wake testing framework.

Throughout the review, we focused on:

verifying the logic against specifications,
ensuring assets cannot be locked or lost,
validating the implementation of ERC-3156 flashloans,
confirming compliance with ERC-4337 restrictions,
identifying potential reentrancy vulnerabilities in the code,
verifying the accuracy of the system’s arithmetic,
ensuring access controls are appropriately balanced,
scanning for common issues like data validation errors.

SCOPE

The audit was conducted on the commit 013a123 and the specific scope included the following files:

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

FINDINGS

Below are the findings from our review.

Critical severity

No issues of critical severity were identified.

High severity

H1: Absence of threshold checks

H2: Incorrect removal from an array of signatures in removeSigHook

H3: Locked Ether in OwnableExecutor

H4: Restricted storage access according to ERC-4337

H5: Limited access for Nominee

H6: Borrower’s nonce can be externally increased

H7: Issues with ERC-3156 flashloans implementation

Medium severity

M1: Missing check for sqrtPriceLimitX96

M2: Removal of an address from a different context

M3: Lack of module type condition

Low severity

L1: HookMultiPlexer without any hooks

L2: Front-running vulnerability in flashLoan

L3: Unsafe ERC-20 interactions

L4: Missing initialized checks in SentinelList

L5: Execution element deletion is not managed

L6: List element exclusion issues

Warning severity

W1: Duplicate validators in MultiFactor

W2: Absent call to clearTrustedForwarder

W3: Validation of execution count in SchedulingBase

W4: Absence of zero address validation

W5: Missing array length checks

W6: Lack of value checks in ERC-20 transfers

W7: TODOs present in module HookMultiPlexer

Informational severity

I1: Precision issues in AutoSavings percentage

I2: Code segments that are unused

I3: Variables that remain unused

I4: Internal functions lack a prefix

I5: Events that are missing

I6: Typographical errors and incorrect documentation

I7: Redundant assignments noted in SentinelList

I8: Function access restrictions that are incomplete

I9: Recommendation for refactoring HookMultiPlexer

CONCLUSION

Our audit produced 32 findings across all severity levels from Informational to High.

The most significant high-severity findings pertain to various coding issues, including missing threshold checks (H1), errors in removing hooks from the wrong list (H2), locked Ether (H3), violations of ERC-4337 regarding restricted storage access (H4), problems with waitPeriod for the nominee (H5), increased borrower’s nonce vulnerability (H6), and multiple compliance issues in ERC-3156 flashloans implementation (H7). Given the substantial issues in the codebase, we advise against deploying or using the contracts until all high-severity problems are rectified. While the codebase is generally well documented, its quality doesn’t meet the expected standards of polished reference examples.

Ackee Blockchain recommends that Rhinestone should:

implement threshold protection when removing validators/owners,
avoid locking assets within the contract,
ensure compliance with ERC-4337 regarding restricted storage slots,
correct the lastAccess timestamp reset for nominees in DeadmanSwitch contract,
address bypassing whitelist and nonce issues in ColdStorageFlashloan contract,
strictly adhere to the ERC-3156 specifications,
integrate slippage protection checks within the ScheduledOrders contract,
fix the parameter order in SentinelList.pop function in ColdStorageFlashLoan.removeAddress ,
correct module type conditions in ColdStorageHook function,
resolve all other identified issues,
conduct a comprehensive internal code review to enhance code quality,
complete any outstanding documentation.

Ackee Blockchain’s complete audit report for Rhinestone, including a detailed account of all findings and suggested improvements, can be accessed here.

We were pleased to conduct the audit for Rhinestone and look forward to future collaborations.