Rhinestone ERC-7579 Safe Adapter Audit Summary

Rhinestone’s ERC-7579 adapter for Safe smart accounts provides them with full ERC-4337 and ERC-7579 compliance, which it achieves by serving as Safe’s Fallback handler as well as an enabled module. In turn, this allows all Safe smart accounts to utilize all ERC-7579 modules. The adapter can be added to already existing Safe smart accounts; however, a launchpad was also developed to enable setup of new Safe smart accounts with the ERC-7579 adapter already enabled.

Rhinestone engaged Ackee Blockchain Security to perform a security review of the Rhinestone ERC-7570 adapter for Safe smart accounts for a total of 16 engineering days in a period between June 3 and June 14, 2024.

Rhinestone also engaged Ackee Blockchain Security to perform an incremental security review of an updated version of the Safe7579 module with a total time donation of 3 engineering days in a period between July 2 and July 5, 2024,

METHODOLOGY

We began our review using static analysis tools, including Wake in companion with Solidity (Wake) 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 Safe deployment using the Launchpad,
• checking module management logic and multi-type module installation,
• checking fallback handler implementation,
• checking possible DoS scenarios,
• checking front-running possibilities,
• ensuring delegatecalls are used correctly,
• detecting possible reentrancies in the code,
• checking compliance with used ERCs,
• ensuring access controls are not too relaxed or too strict,
• looking for common issues such as data validation.

SCOPE

The audit was performed on the commit 90dd363 and the scope was the following:

• core/
  • AccessControl.sol
  • ExecutionHelper.sol
  • Initializer.sol
  • ModuleManager.sol
  • RegistryAdapter.sol
  • SetupDCUtil.sol
• lib/
  • ExecutionLib.sol
  • ModeLib.sol
• utils/
  • DCUtil.sol
  • Safe7579UserOperationBuilder.sol
• DataTypes.sol
• Safe7579.sol
• Safe7579Launchpad.sol

FINDINGS

Here are the findings from our audit.

Critical severity

C1: ERC-4337 counterfactual address can be stolen

High severity

H1: initializeAccount vulnerable to front-running

H2: Executors cannot be used

Medium severity

M1: Missing event and onInstall call in _initModules

M2: BatchedExecUtil._tryExecute inverted success

M3: BatchedExecUtil.tryExecute single return value

M4: ModuleManager._installHook SIG hook overwriting

M5: Locked Ether

Low severity

L1: Fallback handler CallType validation

L2: Domain-specific message encoding missing with signedMessages

L3: ERC-4337 factory standard violation

L4: _multiTypeInstall module type validation

Warning severity

W1: postCheck function is different from the EIP-7579 interface

W2: uninstallModule reverts on multi-type module

W3: Hooks can prevent module uninstallation

W4: Missing data validations

W5: Underscore prefixed public function

W6: Hardcoded Enum.Operation values

W7: Incomplete unused Safe7579UserOperationBuild er

W8: Missing TryExecutionFailed emits

Information severity

I1: Duplicated code

I2: Unused code

I3: Typos and incorrect documentation

I4: Code structure

W9: Safe does not implement validator interface

W10: Inconsistent signature checking

I5: Unused using-for

I6: Typo

CONCLUSION

Our review resulted in 28 findings, ranging from Informational to Critical severity. The most severe one allows the attacker to front-run the Safe deployment using the Launchpad and take over control of smart wallets created using it (see C1). Other high-severity issues refer to the Safe7579.initializeAccount function front-running (H1) and the wrong context used in the withRegistry modifier in the Safe7579.executeFromExecutor function (H2). Medium issues are mostly overlooked trivial mistakes. The overall code quality is average, the codebase contains TODOs, the unused code and also the project is not fully covered by NatSpec documentation. 

 

Ackee Blockchain Security recommends Rhinestone:

• fix newly deployed Safe takeover possibility,
• protect the Safe7579.initializeAccount function from front-running,
• fix the context in the withRegistry modifier in Safe7579.executeFromExecutor function,
• fix SIG hooks overriding,
• fix issues with the success return values in batch execution,
• call module onInstall function during _initModule process,
• resolve all TODOs and remove unused code,
• cover the codebase with NatSpec documentation,
• address all other reported issues,
• also, we recommend performing continuous internal peer code reviews.

 

Ackee Blockchain Security’s full Rhinestone audit report, which includes 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.