{"id":910,"date":"2024-08-21T12:09:45","date_gmt":"2024-08-21T10:09:45","guid":{"rendered":"https:\/\/ackee.xyz\/blog\/?p=910"},"modified":"2024-08-21T12:09:45","modified_gmt":"2024-08-21T10:09:45","slug":"rhinestone-core-modules-audit-summary","status":"publish","type":"post","link":"https:\/\/ackee.xyz\/blog\/rhinestone-core-modules-audit-summary\/","title":{"rendered":"Rhinestone Core Modules Audit Summary"},"content":{"rendered":"<p><span style=\"font-weight: 400;\">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:<\/span><\/p>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">AutoSavings: Automatically save a portion of received tokens into a vault<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">ColdStorageHook: Secure your account by transforming it into cold storage for your assets<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">ColdStorageFlashloan: Enable using the utility of your assets in cold storage through flashloans<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">DeadmanSwitch: Secure your account by setting up a deadman switch<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">HookMultiPlexer: Use multiple hooks based on granular conditions<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">MultiFactor: Multiplex different validators to make your account more secure<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">OwnableExecutor: Control your account from another account<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">OwnableValidator: Own your account using an EOA or a set of EOAs<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">RegistryHook: Use the Rhinestone Registry to ensure you only install secure modules<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">ScheduledOrders: Automate swaps on a schedule<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">ScheduledTransfers: Automate transfers on a schedule<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">SocialRecovery: Recover your account using a set of trusted friends<\/span><\/li>\n<\/ul>\n<p><span style=\"font-weight: 400;\"><a href=\"https:\/\/rhinestone.wtf\/\" target=\"_blank\" rel=\"noopener\">Rhinestone<\/a> 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.<\/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 Wake in companion with <a href=\"https:\/\/marketplace.visualstudio.com\/items?itemName=AckeeBlockchain.tools-for-solidity\" target=\"_blank\" rel=\"noopener\">Tools for Solidity<\/a> VS Code extension. We then took a deep dive into the logic of the contracts. For testing and fuzzing, we have involved <a href=\"https:\/\/getwake.io\/\" target=\"_blank\" rel=\"noopener\">Wake testing framework<\/a>.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">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 the logic of examples according to specifications,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">checking the assets cannot be locked or lost,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">validating ERC-3156 flashloans implementation,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">checking ERC-4337 restrictions are followed,<\/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;\">ensuring the arithmetic of the system is correct,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">ensuring access controls are not too weak or too strict,\u00a0<\/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 has been performed on the commit <code class=\"codehl\">013a123<\/code> and the exact scope was the following files:<\/span><\/p>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">Core Modules, excluding external dependencies,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">SentinelList library (<code class=\"codehl\">f3f84d6<\/code>),<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">CheckNSignatures library (<code class=\"codehl\">53617ec<\/code>).<\/span><\/li>\n<\/ul>\n<h2><span style=\"font-weight: 400;\">FINDINGS<\/span><\/h2>\n<p><span style=\"font-weight: 400;\">Here we present our findings.<\/span><\/p>\n<h3><span style=\"font-weight: 400;\">Critical severity<\/span><\/h3>\n<p><span style=\"font-weight: 400;\">No critical severity issues were found.\u00a0<\/span><\/p>\n<h3><span style=\"font-weight: 400;\">High severity<\/span><\/h3>\n<p><span style=\"font-weight: 400;\">H1: Missing threshold checks<\/span><\/p>\n<p><span style=\"font-weight: 400;\">H2: Removing from a wrong array of sigs in <code class=\"codehl\">removeSigHook<\/code> <\/span><\/p>\n<p><span style=\"font-weight: 400;\">H3: <code class=\"codehl\">OwnableExecutor<\/code> locked Ether<\/span><\/p>\n<p><span style=\"font-weight: 400;\">H4: ERC-4337 restricted storage access<\/span><\/p>\n<p><span style=\"font-weight: 400;\">H5: <code class=\"codehl\">Nominee<\/code> have limited access<\/span><\/p>\n<p><span style=\"font-weight: 400;\">H6: Externally increasable borrower&#8217;s nonce<\/span><\/p>\n<p><span style=\"font-weight: 400;\">H7: ERC-3156 flashloans implementation<\/span><\/p>\n<h3><span style=\"font-weight: 400;\">Medium severity<\/span><\/h3>\n<p><span style=\"font-weight: 400;\">M1: Missing <code class=\"codehl\">sqrtPriceLimitX96<\/code> check<\/span><\/p>\n<p><span style=\"font-weight: 400;\">M2: Removing different address<\/span><\/p>\n<p><span style=\"font-weight: 400;\">M3: Missing module type condition<\/span><\/p>\n<h3><span style=\"font-weight: 400;\">Low severity<\/span><\/h3>\n<p><span style=\"font-weight: 400;\">L1: <code class=\"codehl\">HookMultiPlexer<\/code> with no hooks<\/span><\/p>\n<p><span style=\"font-weight: 400;\">L2: <code class=\"codehl\">flashLoan<\/code> front-run<\/span><\/p>\n<p><span style=\"font-weight: 400;\">L3: Unsafe ERC-20 calls<\/span><\/p>\n<p><span style=\"font-weight: 400;\">L4: Missing initialized check in SentinelList<\/span><\/p>\n<p><span style=\"font-weight: 400;\">L5: Missing deletion of execution element<\/span><\/p>\n<p><span style=\"font-weight: 400;\">L6: Excluding list element<\/span><\/p>\n<h3><span style=\"font-weight: 400;\">Warning severity<\/span><\/h3>\n<p><span style=\"font-weight: 400;\">W1: <code class=\"codehl\">MultiFactor<\/code> duplicate validators<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W2: Missing <code class=\"codehl\">clearTrustedForwarder<\/code> call<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W3: <code class=\"codehl\">SchedulingBase<\/code> executions count validation<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W4: Missing zero address check<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W5: Missing array length validation<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W6: Missing value check in ERC-20 transfers<\/span><\/p>\n<p><span style=\"font-weight: 400;\">W7: TODOs in module <code class=\"codehl\">HookMultiPlexer<\/code> <\/span><\/p>\n<h3><span style=\"font-weight: 400;\">Informational severity<\/span><\/h3>\n<p><span style=\"font-weight: 400;\">I1: <code class=\"codehl\">AutoSavings<\/code> percentage precision<\/span><\/p>\n<p><span style=\"font-weight: 400;\">I2: Unused code<\/span><\/p>\n<p><span style=\"font-weight: 400;\">I3: Unused variable<\/span><\/p>\n<p><span style=\"font-weight: 400;\">I4: Internal functions missing prefix<\/span><\/p>\n<p><span style=\"font-weight: 400;\">I5: Missing events<\/span><\/p>\n<p><span style=\"font-weight: 400;\">I6: Typos and incorrect documentation<\/span><\/p>\n<p><span style=\"font-weight: 400;\">I7: Redundant assignments in <code class=\"codehl\">SentinelList<\/code> <\/span><\/p>\n<p><span style=\"font-weight: 400;\">I8: Missing function restriction<\/span><\/p>\n<p><span style=\"font-weight: 400;\">I9: Proposal for refactoring <code class=\"codehl\">HookMultiPlexer<\/code> <\/span><\/p>\n<h2><span style=\"font-weight: 400;\">CONCLUSION<\/span><\/h2>\n<p><span style=\"font-weight: 400;\">Our review resulted in 32 findings ranging from Info to High severity.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">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 <code class=\"codehl\">waitPeriod<\/code> for the nominee (H5), externally increasable borrower\u2019s 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.<\/span><\/p>\n<p>&nbsp;<\/p>\n<p><b>Ackee Blockchain recommends Rhinestone to:<\/b><\/p>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">add the threshold protection when removing validators\/owners,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">avoid locking assets in the contract,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">prevent interacting with restricted storage slots according to ERC-4337 rules,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">fix <code class=\"codehl\">lastAccess<\/code> timestamp resetting for a nominee in <code class=\"codehl\">DeadmanSwitch<\/code> contract,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">fix bypassing whitelist and nonce increase in <code class=\"codehl\">ColdStorageFlashloan<\/code> contract,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">strictly follow the ERC-3156 specification,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">add a check for slippage protection in <code class=\"codehl\">ScheduledOrders<\/code> contract,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">fix the <code class=\"codehl\">SentinelList.pop<\/code> function parameters order in <code class=\"codehl\">ColdStorageFlashLoan.removeAddress<\/code> ,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">fix module types condition in <code class=\"codehl\">ColdStorageHook<\/code> function,<\/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;\">perform a complete internal code review to ensure better code quality,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">complete the missing documentation.<\/span><\/li>\n<\/ul>\n<p>&nbsp;<\/p>\n<p><b>Ackee Blockchain\u2019s full Rhinestone audit report with 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-core-modules-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 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&hellip;<\/p>\n","protected":false},"author":17,"featured_media":911,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[20,10,103],"tags":[89,141,101,104],"class_list":["post-910","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-audits","category-ethereum","category-wake","tag-audit-summary","tag-rhinestone","tag-safe","tag-wake"],"aioseo_notices":[],"featured_image_src":"https:\/\/ackee.xyz\/blog\/wp-content\/uploads\/2024\/08\/rhinestone.wtf2_-600x400.png","featured_image_src_square":"https:\/\/ackee.xyz\/blog\/wp-content\/uploads\/2024\/08\/rhinestone.wtf2_-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\/910","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=910"}],"version-history":[{"count":0,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/posts\/910\/revisions"}],"wp:featuredmedia":[{"embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/media\/911"}],"wp:attachment":[{"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/media?parent=910"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/categories?post=910"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/tags?post=910"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}