{"id":950,"date":"2024-10-25T17:11:55","date_gmt":"2024-10-25T15:11:55","guid":{"rendered":"https:\/\/ackee.xyz\/blog\/?p=950"},"modified":"2024-10-25T17:11:55","modified_gmt":"2024-10-25T15:11:55","slug":"rhinestone-erc-7579-safe-adapter-audit-summary","status":"publish","type":"post","link":"https:\/\/ackee.xyz\/blog\/rhinestone-erc-7579-safe-adapter-audit-summary\/","title":{"rendered":"Rhinestone ERC-7579 Safe Adapter Audit Summary"},"content":{"rendered":"<p><span style=\"font-weight: 400;\"><a href=\"https:\/\/erc7579.com\/\" target=\"_blank\" rel=\"noopener\">Rhinestone&#8217;s ERC-7579<\/a> adapter for Safe smart accounts provides them with full ERC-4337 and ERC-7579 compliance, which it achieves by serving as Safe&#8217;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.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">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.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">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,<\/span><\/p>\n<h2><span style=\"font-weight: 400;\">METHODOLOGY<\/span><\/h2>\n<p><span style=\"font-weight: 400;\">We began our review using static analysis tools, including <\/span><a href=\"https:\/\/getwake.io\/\"><span style=\"font-weight: 400;\">Wake<\/span><\/a><span style=\"font-weight: 400;\"> in companion with <\/span><a href=\"https:\/\/marketplace.visualstudio.com\/items?itemName=AckeeBlockchain.tools-for-solidity\"><span style=\"font-weight: 400;\">Solidity (Wake) VS Code extension<\/span><\/a><span style=\"font-weight: 400;\">. 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:<\/span><\/p>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">checking Safe deployment using the Launchpad,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">checking module management logic and multi-type module installation,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">checking fallback handler implementation,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">checking possible DoS scenarios,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">checking front-running possibilities,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">ensuring delegatecalls are used correctly,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">detecting possible reentrancies in the code,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">checking compliance with used ERCs,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">ensuring access controls are not too relaxed or too strict,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">looking for common issues such as data validation.<\/span><\/li>\n<\/ul>\n<h2><span style=\"font-weight: 400;\">SCOPE<\/span><\/h2>\n<p><span style=\"font-weight: 400;\">The audit was performed on the commit <code class=\"codehl\">90dd363<\/code> and the scope was the following:<\/span><\/p>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">core\/<\/span>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"2\"><span style=\"font-weight: 400;\">AccessControl.sol<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"2\"><span style=\"font-weight: 400;\">ExecutionHelper.sol<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"2\"><span style=\"font-weight: 400;\">Initializer.sol<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"2\"><span style=\"font-weight: 400;\">ModuleManager.sol<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"2\"><span style=\"font-weight: 400;\">RegistryAdapter.sol<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"2\"><span style=\"font-weight: 400;\">SetupDCUtil.sol<\/span><\/li>\n<\/ul>\n<\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">lib\/<\/span>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"2\"><span style=\"font-weight: 400;\">ExecutionLib.sol<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"2\"><span style=\"font-weight: 400;\">ModeLib.sol<\/span><\/li>\n<\/ul>\n<\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">utils\/<\/span>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"2\"><span style=\"font-weight: 400;\">DCUtil.sol<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"2\"><span style=\"font-weight: 400;\">Safe7579UserOperationBuilder.sol<\/span><\/li>\n<\/ul>\n<\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">DataTypes.sol<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">Safe7579.sol<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">Safe7579Launchpad.sol<\/span><\/li>\n<\/ul>\n<h2><span style=\"font-weight: 400;\">FINDINGS<\/span><\/h2>\n<p><span style=\"font-weight: 400;\">Here are the findings from our audit.<\/span><\/p>\n<h3><span style=\"font-weight: 400;\">Critical severity<\/span><\/h3>\n<p><span style=\"font-weight: 400;\">C1: ERC-4337 counterfactual address can be stolen<\/span><\/p>\n<h3><span style=\"font-weight: 400;\">High severity<\/span><\/h3>\n<p><span style=\"font-weight: 400;\">H1: <code class=\"codehl\">initializeAccount<\/code> vulnerable to front-running<\/span><\/p>\n<p><span style=\"font-weight: 400;\">H2: Executors cannot be used<\/span><\/p>\n<h3><span style=\"font-weight: 400;\">Medium severity<\/span><\/h3>\n<p><span style=\"font-weight: 400;\">M1: Missing event and <code class=\"codehl\">onInstall<\/code> call in <code class=\"codehl\">_initModules<\/code><\/span><\/p>\n<p><span style=\"font-weight: 400;\">M2: <code class=\"codehl\">BatchedExecUtil._tryExecute<\/code> inverted <code class=\"codehl\">success<\/code><\/span><\/p>\n<p><span style=\"font-weight: 400;\">M3: <code class=\"codehl\">BatchedExecUtil.tryExecute<\/code> single return value<\/span><\/p>\n<p><span style=\"font-weight: 400;\">M4: <code class=\"codehl\">ModuleManager._installHook<\/code> SIG hook overwriting<\/span><\/p>\n<p><span style=\"font-weight: 400;\">M5: Locked Ether<\/span><\/p>\n<h3><span style=\"font-weight: 400;\">Low severity<\/span><\/h3>\n<p><span style=\"font-weight: 400;\">L1: Fallback handler <code class=\"codehl\">CallType<\/code> validation<\/span><\/p>\n<p><span style=\"font-weight: 400;\">L2: Domain-specific message encoding missing with <code class=\"codehl\">signedMessages<\/code><\/span><\/p>\n<p><span style=\"font-weight: 400;\">L3: ERC-4337 factory standard violation<\/span><\/p>\n<p><span style=\"font-weight: 400;\">L4: <code class=\"codehl\">_multiTypeInstall<\/code> module type validation<\/span><\/p>\n<h3><span style=\"font-weight: 400;\">Warning severity<\/span><\/h3>\n<p><span style=\"font-weight: 400;\">W1: <code class=\"codehl\">postCheck<\/code> function is different from the EIP-7579 interface<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W2: <code class=\"codehl\">uninstallModule<\/code> reverts on multi-type module<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W3: Hooks can prevent module uninstallation<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W4: Missing data validations<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W5: Underscore prefixed public function<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W6: Hardcoded Enum.Operation values<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W7: Incomplete unused Safe7579UserOperationBuild er<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W8: Missing <code class=\"codehl\">TryExecutionFailed<\/code> emits<\/span><\/p>\n<h3><span style=\"font-weight: 400;\">Information severity<\/span><\/h3>\n<p><span style=\"font-weight: 400;\">I1: Duplicated code<\/span><\/p>\n<p><span style=\"font-weight: 400;\">I2: Unused code<\/span><\/p>\n<p><span style=\"font-weight: 400;\">I3: Typos and incorrect documentation<\/span><\/p>\n<p><span style=\"font-weight: 400;\">I4: Code structure<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W9: Safe does not implement validator interface<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W10: Inconsistent signature checking<\/span><\/p>\n<p><span style=\"font-weight: 400;\">I5: Unused using-for<\/span><\/p>\n<p><span style=\"font-weight: 400;\">I6: Typo<\/span><\/p>\n<h2><span style=\"font-weight: 400;\">CONCLUSION<\/span><\/h2>\n<p><span style=\"font-weight: 400;\">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 <\/span><span style=\"font-weight: 400;\">C1<\/span><span style=\"font-weight: 400;\">). Other high-severity issues refer to the <\/span><span style=\"font-weight: 400;\">Safe7579.initializeAccount <\/span><span style=\"font-weight: 400;\">function front-running (<\/span><span style=\"font-weight: 400;\">H1<\/span><span style=\"font-weight: 400;\">) and the wrong context used in the <\/span><span style=\"font-weight: 400;\">withRegistry <\/span><span style=\"font-weight: 400;\">modifier in the <\/span><span style=\"font-weight: 400;\">Safe7579.executeFromExecutor <\/span><span style=\"font-weight: 400;\">function (<\/span><span style=\"font-weight: 400;\">H2<\/span><span style=\"font-weight: 400;\">). 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.\u00a0<\/span><\/p>\n<p>&nbsp;<\/p>\n<p><b>Ackee Blockchain Security recommends Rhinestone:<\/b><\/p>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">fix newly deployed Safe takeover possibility,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">protect the <code class=\"codehl\">Safe7579.initializeAccount<\/code> function from front-running,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">fix the context in the <code class=\"codehl\">withRegistry<\/code> modifier in <code class=\"codehl\">Safe7579.executeFromExecutor<\/code> function,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">fix SIG hooks overriding,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">fix issues with the <code class=\"codehl\">success<\/code> return values in batch execution,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">call module <code class=\"codehl\">onInstall<\/code> function during <code class=\"codehl\">_initModule<\/code> process,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">resolve all TODOs and remove unused code,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">cover the codebase with NatSpec documentation,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">address all other reported issues,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">also, we recommend performing continuous internal peer code reviews.<\/span><\/li>\n<\/ul>\n<p>&nbsp;<\/p>\n<p><b>Ackee Blockchain Security\u2019s full Rhinestone audit report, which includes a more detailed description of all findings and recommendations, can be found <a href=\"https:\/\/github.com\/Ackee-Blockchain\/public-audit-reports\/blob\/master\/2024\/ackee-blockchain-rhinestone-safe7579-report.pdf\" target=\"_blank\" rel=\"noopener\">here<\/a>.<\/b><\/p>\n<p>&nbsp;<\/p>\n<p><span style=\"font-weight: 400;\">We were delighted to audit Rhinestone and look forward to working with them again.<\/span><\/p>\n","protected":false},"excerpt":{"rendered":"<p>Rhinestone&#8217;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&#8217;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&hellip;<\/p>\n","protected":false},"author":17,"featured_media":898,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[20,10,103],"tags":[89,146,145,141,101,104],"class_list":["post-950","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-audits","category-ethereum","category-wake","tag-audit-summary","tag-erc","tag-erc-7579","tag-rhinestone","tag-safe","tag-wake"],"aioseo_notices":[],"featured_image_src":"https:\/\/ackee.xyz\/blog\/wp-content\/uploads\/2024\/08\/rhinestone.wtf-2-600x400.png","featured_image_src_square":"https:\/\/ackee.xyz\/blog\/wp-content\/uploads\/2024\/08\/rhinestone.wtf-2-600x600.png","author_info":{"display_name":"Stepan Sonsky","author_link":"https:\/\/ackee.xyz\/blog\/author\/stepan\/"},"_links":{"self":[{"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/posts\/950","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/users\/17"}],"replies":[{"embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/comments?post=950"}],"version-history":[{"count":0,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/posts\/950\/revisions"}],"wp:featuredmedia":[{"embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/media\/898"}],"wp:attachment":[{"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/media?parent=950"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/categories?post=950"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/tags?post=950"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}