Rhinestone is dedicated to transforming smart accounts into an open platform for innovation. They aim to create a secure and interoperable ecosystem of modules, fostering permissionless smart account innovation and enabling developers to build powerful onchain products with seamless UX.
The Module Registry is a foundational layer of Rhinestone’s tech stack. Its primary function is to enforce standards and security guarantees for users and developers by storing onchain security assertions made by independent auditors. The registry provides a scalable and trustless system for solving smart account security and enabling an open module ecosystem that is account vendor-agnostic, thanks to ERC-7579.
Rhinestone engaged Ackee Blockchain to perform a security review of the Rhinestone Module Registry for a total of 7 engineering days in a period between April 10 and April 19, 2024.
METHODOLOGY
We began our review by using static analysis tools, namely 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 module deployment can not be misused,
- detecting possible reentrancies in the code,
- checking possible front-running,
- checking for denial of service attacks,
- ensuring access controls are not too relaxed or too strict,
- looking for common issues such as data validation.
SCOPE
The audit has been performed on the commit 6f5e84a and the exact scope was the following files:
- ./core/Attestation.sol
- ./core/AttestationManager.sol
- ./core/ModuleManager.sol
- ./core/ResolverManager.sol
- ./core/SchemaManager.sol
- ./core/SignedAttestation.sol
- ./core/TrustManager.sol
- ./core/TrustManagerExternalAttesterList.sol
- ./lib/AttestationLib.sol
- ./lib/Helpers.sol
- ./lib/ModuleDeploymentLib.sol
- ./lib/ModuleTypeLib.sol
- ./lib/StubLib.sol
- ./lib/TrustLib.sol
- ./Common.sol
- ./DataTypes.sol
- ./Registry.sol
FINDINGS
Here we present our findings.
Critical severity
No critical severity issues were found.
High severity
H1: threshold = 1 optimization DoS
Medium severity
M1: Arbitrary call on factory
M2: Attesters are not de-duplicated
M3: registerModule front-running
M4: trustAttesters downcast
Low severity
L1: Resolver one-step ownership transfer
Warning severity
W1: Deployment and attestation denial of service
W2: Inconsistent revert errors
W3: EIP-712 compliance
W4: findTrustedAttesters revert on no attesters
W5: trustAttesters zero address validation
W6: Inconsistent data validation
W7: TrustLib high-order bits not cleared
Information severity
I1: Multiple interfaces
I2: Inconsistent parameter naming
I3: Duplicated code
I4: Modifier placement
I5: Missing NatSpec documentation
I6: _storeAttestation false comment
I7: NewTrustedAttesters event
CONCLUSION
Our review resulted in 20 findings ranging from Info to High severity.
Ackee Blockchain recommends Rhinestone to:
- remove the optimization for
threshold = 1, - separate the factory logic to the neutral contract,
- resolve the risk of front-running,
- implement two-step ownership transfer,
- address all other reported issues.
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.
