{"id":522,"date":"2023-06-26T08:27:23","date_gmt":"2023-06-26T06:27:23","guid":{"rendered":"https:\/\/ackeeblockchain.com\/blog\/?p=522"},"modified":"2024-07-11T15:32:26","modified_gmt":"2024-07-11T13:32:26","slug":"safe-contracts-1-4-0-audit-summary","status":"publish","type":"post","link":"https:\/\/ackee.xyz\/blog\/safe-contracts-1-4-0-audit-summary\/","title":{"rendered":"Safe: Contracts 1.4.0 audit summary"},"content":{"rendered":"<p><a href=\"https:\/\/safe.global\/\" target=\"_blank\" rel=\"noopener\"><span style=\"font-weight: 400;\">Safe<\/span><\/a><span style=\"font-weight: 400;\"> is a decentralized custody protocol allowing multi-signature (multi-sig) wallets to be used as a single account. Businesses and individuals can use multi-sig wallets for safe collective management, perform sensitive transactions, and achieve redundancy. The protocol is widely used across the Ethereum and EVM ecosystems.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">Safe engaged Ackee Blockchain to perform a security review of the <\/span><b>Safe contracts version 1.4.0 <\/b><span style=\"font-weight: 400;\">with a total time donation of <\/span><b>10<\/b><span style=\"font-weight: 400;\"><strong> engineering days<\/strong> in a period between <\/span><b>February 27<\/b><span style=\"font-weight: 400;\"> and <\/span><b>March 10, 2023.\u00a0<\/b><\/p>\n<h2><b>METHODOLOGY<\/b><\/h2>\n<p><span style=\"font-weight: 400;\">We began our review using static analysis tool <\/span><a href=\"https:\/\/getwake.io\/\" target=\"_blank\" rel=\"noopener\"><span style=\"font-weight: 400;\">Wake<\/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 <\/span><span style=\"font-weight: 400;\">Wake <\/span><span style=\"font-weight: 400;\">testing framework where we simulated deployment of the Safe and focused on the correctness of signature and owner handling.<\/span><\/p>\n<p><span style=\"font-weight: 400;\">During the review, we paid particular attention to: <\/span><\/p>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">signature validation<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">malicious owner actions<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">modules handling<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">owners handling<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">guard handling<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">fallback handler logic<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">access controls<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">delegate call risks<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">data validation. <\/span><\/li>\n<\/ul>\n<h2><b>SCOPE\u00a0<\/b><\/h2>\n<p><span style=\"font-weight: 400;\">The audit has been performed on the commit <\/span><code class=\"codehl\">eb93dbb<\/code><span style=\"font-weight: 400;\">, and the scope was the following contracts with all imports (recursively):<\/span><\/p>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">SafeL2.sol<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">proxies\/SafeProxyFactory.sol<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">handler\/CompatibilityFallbackHandler.sol <\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">libraries\/MultiSendCallOnly.sol<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">libraries\/SignMessageLib.sol<\/span><\/li>\n<\/ul>\n<p><span style=\"font-weight: 400;\">The review was done on March 28 on the given commit <code class=\"codehl\">cb4b2b1<\/code><span style=\"font-weight: 400;\">.<\/span><\/span><\/p>\n<h2><b>FINDINGS<\/b><\/h2>\n<p><span style=\"font-weight: 400;\">Here we present our <\/span><span style=\"font-weight: 400;\">findings<\/span><span style=\"font-weight: 400;\">.<\/span><\/p>\n<h3><b>Critical severity\u00a0<\/b><\/h3>\n<p><span style=\"font-weight: 400;\">No critical severity issues were found.\u00a0<\/span><\/p>\n<h3><b>High severity\u00a0<\/b><\/h3>\n<p><span style=\"font-weight: 400;\">No high severity issues were found.\u00a0<\/span><\/p>\n<h3><b>Medium severity<\/b><\/h3>\n<p><b>M1: <\/b><span style=\"font-weight: 400;\">Broken guard can cause DoS\u00a0<\/span><\/p>\n<p><b>M2: <\/b><span style=\"font-weight: 400;\">Lack of contract check <\/span> <span style=\"font-weight: 400;\">\u00a0<\/span><\/p>\n<h3><b>Low severity<\/b><\/h3>\n<p><b>L1: <\/b><span style=\"font-weight: 400;\">Error-prone proxy constructor\u00a0<\/span><\/p>\n<h3><b>Warning severity <\/b><\/h3>\n<p><b>W1: <\/b><span style=\"font-weight: 400;\">Usage of delegatecalls <\/span><\/p>\n<p><b>W2:<\/b><span style=\"font-weight: 400;\"> Fallback handler can be set to address(this) <\/span><\/p>\n<p><b>W3:<\/b><span style=\"font-weight: 400;\"> Removed owner&#8217;s stored hash <\/span><\/p>\n<p><b>W4:<\/b><span style=\"font-weight: 400;\"> Singleton address at slot 0 <\/span><\/p>\n<p><b>W5: <\/b><span style=\"font-weight: 400;\">Call to disableModule can be frontrun\u00a0<\/span><\/p>\n<p><b>W6: <\/b><span style=\"font-weight: 400;\">Threshold can be set too high <\/span><\/p>\n<h3><b>Informational severity <\/b><\/h3>\n<p><b>I1: <\/b><span style=\"font-weight: 400;\">Code and comment inconsistency\u00a0<\/span><\/p>\n<p><b>I2: <\/b><span style=\"font-weight: 400;\">Require should be assert <\/span><\/p>\n<h2><b>CONCLUSION<\/b><\/h2>\n<p><span style=\"font-weight: 400;\">Our review resulted in <\/span><b>11 findings<\/b><span style=\"font-weight: 400;\">, ranging from <\/span><i><span style=\"font-weight: 400;\">Info<\/span><\/i><span style=\"font-weight: 400;\"> to <\/span><i><span style=\"font-weight: 400;\">Medium<\/span><\/i><span style=\"font-weight: 400;\"> severity. The quality of the code is exceptional. NatSpec in-code documentation is part of every contract and function. General documentation still needs to be created, but Safe team provided a few documents describing the most crucial part &#8211; signatures.\u00a0<\/span><\/p>\n<p><span style=\"font-weight: 400;\">We recommended Safe to:<\/span><\/p>\n<ul>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">change guard management logic,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">mitigate impacts of malicious deployer,<\/span><\/li>\n<li style=\"font-weight: 400;\" aria-level=\"1\"><span style=\"font-weight: 400;\">address all other reported issues.\u00a0<\/span><\/li>\n<\/ul>\n<p><b>Ackee Blockchain\u2019s full <\/b><b><i>Safe <\/i><\/b><b>audit report with a more detailed description of all findings and recommendations can be found <\/b><a href=\"https:\/\/github.com\/safe-global\/safe-contracts\/blob\/e870f514ad34cd9654c72174d6d4a839e3c6639f\/docs\/Safe_Audit_Report_1_4_0.pdf\" target=\"_blank\" rel=\"noopener\"><b>here<\/b><\/a><b>.<\/b><\/p>\n<p><span style=\"font-weight: 400;\">We were delighted to audit<\/span><b> Safe<\/b><span style=\"font-weight: 400;\"> and look forward to working with them again.<\/span><\/p>\n","protected":false},"excerpt":{"rendered":"<p>Safe is a decentralized custody protocol allowing multi-signature (multi-sig) wallets to be used as a single account. Businesses and individuals can use multi-sig wallets for safe collective management, perform sensitive transactions, and achieve redundancy. The protocol is widely used across the Ethereum and EVM ecosystems. Safe engaged Ackee Blockchain to perform a security review of the Safe contracts version 1.4.0 with a&hellip;<\/p>\n","protected":false},"author":15,"featured_media":523,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[20,10,80,103],"tags":[21,24,101,104],"class_list":["post-522","post","type-post","status-publish","format-standard","has-post-thumbnail","hentry","category-audits","category-ethereum","category-solidity","category-wake","tag-audit","tag-ethereum","tag-safe","tag-wake"],"aioseo_notices":[],"featured_image_src":"https:\/\/ackee.xyz\/blog\/wp-content\/uploads\/2023\/06\/Gnosis-audit-600x400.png","featured_image_src_square":"https:\/\/ackee.xyz\/blog\/wp-content\/uploads\/2023\/06\/Gnosis-audit-600x600.png","author_info":{"display_name":"Aleksandra Yudina","author_link":"https:\/\/ackee.xyz\/blog\/author\/aleksandra-yudina\/"},"_links":{"self":[{"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/posts\/522","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\/15"}],"replies":[{"embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/comments?post=522"}],"version-history":[{"count":0,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/posts\/522\/revisions"}],"wp:featuredmedia":[{"embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/media\/523"}],"wp:attachment":[{"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/media?parent=522"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/categories?post=522"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/ackee.xyz\/blog\/wp-json\/wp\/v2\/tags?post=522"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}