深度回顾:发现、攻击场景与今日演进)
OpenZeppelin Contracts 首次安全审计2017 年 3 月深度回顾发现、攻击场景与今日演进【免费下载链接】openzeppelin-contractsOpenZeppelin Contracts is a library for secure smart contract development.项目地址: https://gitcode.com/GitHub_Trending/op/openzeppelin-contracts导读本文完整解读 OpenZeppelin Contracts 历史上第一份由外部机构执行的安全审计报告——2017 年 3 月由 New Alchemy审计人 Dennis Peterson 与 Peter Vessenes针对当时位于zeppelin-solidity仓库commit9c5975a对应 v1.0.4的全部合约所进行的逐行审查。文中将还原审计方识别出的两处严重缺陷CrowdsaleToken 卡死募集资金、MultisigWallet 递归调用可被抽干合约、若干中等问题以及覆盖全部合约模块的逐行评论并对照当前仓库源码contracts/access/Ownable.sol、contracts/token/ERC20/ERC20.sol、contracts/utils/math/Math.sol 等说明这些发现如何在近十年的演进中被逐一解决。读完本文你将理解安全审计报告的标准读法、当时 Solidity 语言生态的关键缺陷以及现代 OpenZeppelin Contracts 中revert 优先算术溢出防护重入防护等设计惯例的历史由来。审计背景、范围与版本依据本次审计由 Zeppelin 团队委托 New Alchemy 执行目标是对其 OpenZeppelin 合约库进行一次独立安全审查。审计的核心理念在报告引言中写得很清楚这些合约被设计为供可能不如 OpenZeppelin 团队熟练的各方安全使用的构建块其设计目标是可以被安全地、原样as-is部署。报告记录了如下关键审计元数据项目内容审计时间2017 年 3 月审计机构New Alchemy审计人Dennis Peterson、Peter Vessenes审计范围仓库contracts目录下全部合约被审计提交9c5975a706b076b7000e8179f8101e0c61024c87对应版本v1.0.4据 audits/README.md 中的审计记录表结论发现 2 个严重错误、1 个中等问题在该提交修复前不建议公开使用这份审计记录至今仍被保留在仓库的 audits/README.md 审计清单第一行与 2018 年 LevelK、2022 年之后的历次 OpenZeppelin 自审计形成连续的安全演进时间线。报告同时声明了审计的边界它不对代码的效用、安全性、商业模式的合规性做任何保证也不保证代码无 bug——这是一份供讨论使用的技术文档。执行摘要高质量代码与两处致命缺陷审计方对当时的 OpenZeppelin 代码库总体评价为质量相当高——代码整洁、模块化、全程遵循最佳实践。但同时也指出三点隐忧代码库仍处于变动期in flux每个文件缺少对预期行为与未来计划的充分文档测试需要更全面、更激进——报告原文甚至建议由不如 OpenZeppelin 团队友善的人来写测试仓库虽已包含一套 Truffle 单元测试报告认为这是此类合约的必要条件与最佳实践但需要增补。在此基础上审计方在该 commit 上发现了两处严重错误Critical和一处中等问题Moderate因此明确建议该提交修复完成之前不应将其用于公开部署。严重问题一CrowdsaleToken 募集资金永久卡死第一个严重问题位于CrowdsaleToken.sol该合约是一个按固定价格铸造代币的 StandardToken用户在发送 ether 时获得代币但合约没有任何函数允许所有者提取募集到的 ether。审计方措辞非常强硬没有任何场景下应该有人原样部署这个合约无论测试还是生产。其建议是作为众筹示例合约应继承 Ownable 并提供标准的withdraw函数由所有者提取 ether而不是让资金永远滞留在合约中报告还提出了一种更优雅的替代模式提供仅可由独立众筹合约调用的mint()函数从而在不修改代币本身的前提下把任何规则价格、上限、时间窗等放进众筹合约代币与资金逻辑分离。这一代币铸造与资金收集分离的思路正是后来 OpenZeppelin 众筹/铸造扩展中_mint由子合约控制、资金由独立合约管理这一设计哲学的雏形。在今天的 ERC20.sol 中可以看到_mint被设计为 internal 函数由派生合约决定铸造时机与调用者外部规则被隔离在派生合约层。严重问题二MultisigWallet 的递归调用可抽干合约第二个严重问题针对MultisigWallet.sol第 45 行——execute在发送金额前会检查当日限额daily limit。攻击面分析如下execute只能由Owner调用问题在于如果多签钱包的所有者批准了一个调用resetSpentToday重置当日已花费额度的提案就能重置每日限额若能构造调用链所有者确认resetSpentToday随后通过execute在递归调用中提款合约资金可被抽干更甚者报告指出甚至不需要递归调用——只需反复交替执行execute与confirm调用即可达到同样效果。审计方当时还在核查Shareable.sol的确认协议虽未 100% 断定该攻击可行但判断看起来是可能的并列出该 bug 的多个成因resetSpentToday与confirm组合起来既不限制可调用的日期也看起来不限制调用次数一个调用被确认并执行之后似乎可以被再次执行缺少已执行标记confirmandCheck似乎没有判断目标函数是否已被调用过的逻辑即便有revoke也需要补充在函数执行完成后的撤销处理逻辑。在MultisigWallet.sol的其他逐行评论中审计方还注意到kill、setDailyLimit、resetSpentToday虽然需要多签批准且 Shareable 会记录其哈希但它们应该再发布自己的事件以便链上轻松读取以及clearPending()被拆分在 MultisigWallet 与 Shareable 两个合约之间虽然这允许继承 Shareable 的合约对待处理交易使用自定义结构体。这个问题的本质是重入/重复执行与状态检查缺失的组合。现代仓库对此类问题的标准答案清晰可见多签治理逻辑已演进为基于角色的 AccessControl.sol角色哈希、grantRole/revokeRole见 contracts/access/AccessControl.sol而重入防护由专门的 ReentrancyGuard.solnonReentrant修饰器基于状态槽标记NOT_ENTERED/ENTERED与先更新状态后转账的检查-效果-交互模式解决。中等问题PullPayment 与 Shareable 的设计缺陷PullPayment缺少取消、溢出与超额排队PullPayment.sol采用收款方主动拉取pull模式本意是安全的但审计方指出它尚不成熟没有显式的付款取消机制考虑收款人丢失钱包、给出恶意地址、或地址回退函数需要超过send默认 gas 等场景取消能力是必要的asyncSend没有溢出检查审计方明确建议在最贴近数据操作的层次做溢出与下溢检查asyncSend允许排队的待付金额超过合约实际余额要么这是坏设计要么至少应该换个名字若有意为之则需要处理多个withdrawPayments调用之间的竞争条件建议增加有多少付款待处理的可见性这需要重写并提醒开发者在当前状态下不要依赖该合约。报告中关于 PullPayment 的一段技术分析对理解以太坊转账语义很有价值asyncSend之后、ether 发送在函数末尾执行且使用.send()而非.call.value()因此对重入攻击是安全的。.call.value()只有在确保所有状态更新之后执行时才更好因为.send在收款方回退函数昂贵时会失败但作为嵌入其他合约的通用工具.send是更稳妥的选择。妥协方案是提供仅所有者可调用的.call.value发送函数。Shareable确认/撤销协议的排序攻击风险审计方对Shareable.sol的结论是尚未为生产环境做好准备缺少函数且按现状可能遭受排序攻击reordering attack——即矿工或与合约参与者竞速的第三方把自身信息插入列表或映射的攻击。报告建议在判定该合约安全之前必须以共享所有者做出极端恶劣行为的假设极其仔细地审查确认与撤销代码同时构造函数对required参数没有健全性检查也令人担忧例如未检查_required len(_owners)若_required被设为MAX - 1这类值会相当糟糕。报告还指出了Shareable.sol的一系列代码质量问题owners、_owners、owner后者来自 Ownable之间仅一个字符的命名差异容易混淆建议重命名第 34 行注释声称本合约只有六种事件实际上只有两种第 61 行ownerIndex为何要把地址哈希成uint再作键而不是直接使用地址以获得更强的类型安全与可读性第 62 行i) ... owners[2 i]让人被迫做数学运算不直观应有添加新操作的封装函数让开发者不必直接操作内部数据结构这也能让多签合约更短存在revoke却没有可见的propose函数若propose允许用户自由选择提案的 bytes 字符串按现状实现会引发不好的事情。逐行审查亮点六大模块的细节发现报告按Lifecycle、Ownership、Payment、Tokens、Root level模块做了逐行评论以下保留其核心结论。LifecycleKillable、Migrations 与 PausableKillable实现非常简单——允许所有者调用selfdestruct并把资金发送给所有者本身无问题。但审计方提醒selfdestruct通常不应使用因为开发者可能日后想访问旧合约中的数据却不理解selfdestruct会永久限制对合约的访问建议补充文档并建议把kill改名为更直白的名字如completelyDestroy而让kill仅负责把资金转给所有者。同时注意可销毁函数允许所有者无视其他逻辑取走资金这在某些场景下是否可取需要权衡。Migrations审计方猜测目标是允许并记录向新合约地址的迁移但当时无法从代码中确认实现方式希望与团队进一步核对。Pausable审计方喜欢暂停机制但指出它给所有者带来显著的恶意破坏griefing潜力且对合约参与者而言可能并不明显。建议在 TokenContract 等示例中加入更安全的 pause/resume 使用样例尤其是建议增加时间锁使任何人都能在超时后解除暂停。这一建议的现代回响可以从 Pausable.sol 看到——如今whenNotPaused/whenPaused修饰器配合显式的EnforcedPause/ExpectedPause错误contracts/utils/Pausable.sol实现检查即回滚而暂停 时间锁的组合模式则沉淀在Pausable与TimelockController等模块的组合用法中。报告特别指出 Pausable 的修饰器使用if(bool){_;}模式这对失败时返回false的函数没问题但对期望抛错的函数则可能有问题——这直接呼应了贯穿全篇的throw vs return false风格问题。OwnershipOwnable 家族与命名建议Ownable第 19 行修饰器在条件不满足时直接 throw与 Pausable 等使用if(bool){_;}的继承式修饰器形成对比——风格不统一是本次审计反复出现的主题。对比今天的 Ownable.solonlyOwner通过_checkOwner()在条件不满足时revert OwnableUnauthorizedAccount(...)并用OwnableInvalidOwner拒绝零地址初始所有者contracts/access/Ownable.sol已完全收敛为抛错式风格。Claimable继承 Ownable现有所有者设置一个pendingOwner由后者主动认领所有权第 17 行是另一个抛错的修饰器。DelayedClaimable审计方质疑它为何直接继承 Ownable 而非 Claimable后者已继承 Ownable认为同时继承二者徒增困惑。Contactable允许所有者设置公开的合约信息字符串无问题。Multisig接口只是一个接口允许更换所有者地址但不允许更改所有者数量这有所限制但也简化了实现。TokensERC20 家族的 approve 竞态与风格分裂ERC20仅标准接口。报告记录了 Edcon 大会披露的标准级安全漏洞approve不防护竞态条件只是覆盖当前值——被授权的 spender 可以等所有者再次调用approve后在新限额生效前花掉旧限额若成功则可花掉两份限额之和。修复途径有二(1) 把旧限额作为参数传入若有部分被花费则更新失败(2) 把 value 参数用作增量delta而非替换值。报告客观指出在严格遵守现有完整 ERC20 标准的前提下无法修复但可以增加secureApprove函数影响有限因为至少只有你批准过的地址才能攻击你用户也可通过先把限额设为 0、确认后再设新限额来缓解。ERC20Basic跳过 Approve 的简化接口且与 ERC20 的另一处偏离是 transfer 抛错而非返回 false。BasicToken使用 SafeSub/SafeMathtransfer 因此抛错而非返回 false符合 ERC20Basic 但不符合真正的 ERC20 标准。StandardToken完整 ERC20 实现。transfer/transferFrom 使用 SafeMath出错时抛错而非返回 false——不是安全问题但偏离标准。SimpleTokenStandardToken 的示例实例decimals 为 18 而总供应量仅 10,000即整个供应量不足一个名义代币。CrowdsaleToken见上文严重问题一。VestedToken第 23、27 行的transfer/transferFrom带有canTransfer修饰器余额不足时抛错但transfer本身返回布尔值——失败处理方式不一致可能给调用该代币的其他合约带来问题transferableTokens()依赖safeSub余额不足时同样抛错第 64 行的delete其实不必要因为下一行就会覆盖该值。Root levelBounty、DayLimit、LimitBalance、MultisigWallet 与 SafeMathBounty通过让每位研究者部署独立合约来进行攻击避免竞争条件——若某研究者攻破了其关联合约其他人不能立刻领取赏金必须在各自合约中复现攻击。但开发者可通过让deployContract()总是返回同一地址来破坏该意图这会更新researchers映射中与合约关联的研究者地址可通过禁止重写researchers来防范。DayLimitlimitedDaily修饰器调用underLimit它既检查当日支出低于限额又把本次输入值累加进spentToday。若所有函数失败时都抛错这是安全的但 OpenZeppelin 并非所有函数都如此——有些返回 false有些用if(bool){_;}包裹函数体此时_value已被计入spentTodayether 却可能因其他前置条件不满足而没有实际发送不过在当时的 multisig 中这不是问题。此外第 4、11 行的注释声称 DayLimit 是多所有者的并导入了 Shareable但 DayLimit 实际并未继承 Shareable——意图可能是让子合约如 Multisig继承这种情况下应删除导入并修正注释。第 46 行手写了溢出检查而没有使用 safeAdd——鉴于该函数本身失败即抛错用 safeAdd 并无坏处。LimitBalance无问题。MultisigWallet除前述严重问题外还有若干细节——kill、setDailyLimit、resetSpentToday需多签批准且 Shareable 记录了哈希但建议各自发布事件便于阅读第 45 行对underLimit的调用会先扣减日限额再抛错或返回 0因此不存在限额被扣减而操作未执行的危险第 65 行 Shareable 的onlyManyOwners会记录用户确认并在足够多用户确认后才执行函数体发送失败时整体抛错并回滚确认确认数不足时返回 false成功时返回 true——审计方评价为优雅的设计第 68 行的 throw 是好的但该函数既可能返回 false 也可能抛错值得注意。SafeMathEdcon 演示中另一个有趣的论点——Solidity 的溢出行为当时未被文档化理论上依赖它的源码可能在未来的编译版本中出错不过编译产物应该没问题且即便编译器真以这种方式修订也会有充分预警这也是把溢出检查隔离在 SafeMath 中的理由。除这一点外审计方认为 SafeMath 本身没有问题。贯穿全篇的两大风格问题错误处理throw 还是 return false这是本次审计最重要的方法论讨论。Solidity 当时允许两种错误处理方式调用throw或返回false。throw保证调用栈直到前一个外部调用被完整回滚false则允许函数继续执行。审计方总体更偏好throw因为它更简单、工程师需要跟踪的状态更少而返回 false 用逻辑检查结果容易演变成难以跟踪的状态机这类复杂性正是错误的温床。当时的 OpenZeppelin 代码库中两种风格并存SimpleToken 转账失败即抛错完整 ERC20 返回 false部分修饰器抛错部分用条件包裹函数体让函数返回 false。审计方并不喜欢这种分裂建议全库统一风格或至少文档化什么场景用哪种技术的设计准则同时客观指出在某些场景下两种方式都不可行——例如 SafeMath 失败时几乎只能抛错而 ERC20 标准规定了返回布尔值因此不给出特定建议只指出不一致之处。这一争论在今天的仓库中已经有了明确答案全面转向 revert抛错。现代 ERC20.sol 的文件头注释直接写明OpenZeppelin Contracts 的惯例是函数失败时 revert 而非返回 false同时通过 draft-IERC6093.sol 定义结构化错误如ERC20InsufficientBalance、ERC20InvalidSender、ERC20InvalidReceiver、ERC20InsufficientAllowance让调用方可以用自定义错误Custom Error精确捕获失败原因而 SafeERC20.sol 则提供safeTransfer、safeTransferFrom、safeIncreaseAllowance、safeDecreaseAllowance等包装把返回 false/无返回值的旧代币统一转为 revert兼容链上的历史代币。Solidity 版本与语言演进审计方注意到当时大部分代码使用 Solidity 0.4.11但Ownership下部分文件仍标记 0.4.0应统一升级。报告前瞻性地列举了 Solidity 0.4.10 将带来的、对这类合约有用的特性assert(condition)条件为假时抛错revert()回滚且不耗尽剩余 gasaddress.transfer(value)类似send但自动传播异常并支持.gas()。这些语言特性正是抛错式错误处理能够大规模落地的基础。现代仓库的 pragma 声明如 Ownable.sol、ERC20.sol 均为^0.8.20表明经过 0.8 版本内置溢出检查与 revert 语义的引入当年审计中讨论的 SafeMath、throw 风格问题已在语言层面得到根治——0.8之后整数运算默认溢出回滚Math.sol 作为现代替代库提供tryAdd/trySub/tryMul带成功标志的非回滚版本以及add512/mul512512 位运算等更精细的工具而非当年失败即抛错的单一 SafeMath 风格。历史回响当年发现如何在今日代码库中演进结合当前仓库源码可以将 2017 年审计的主要结论映射到现代实现构成一条完整的安全演进线索2017 年审计发现今日仓库中的对应答案CrowdsaleToken 资金无法提取铸造逻辑_mint下沉为 internal资金收集与代币逻辑分离ERC20.solMultisigWallet 重入/重复执行风险独立的重入防护模块ReentrancyGuard.sol先更新状态再交互的通用模式Shareable 多签协议脆弱、构造函数缺校验基于角色的 AccessControl.sol角色由bytes32标识、grantRole/revokeRole受 admin 角色约束contracts/access/AccessControl.solthrow 与 return false 风格分裂全库统一 revert 结构化错误draft-IERC6093.sol并用 SafeERC20.sol 兼容历史代币手写溢出检查、SafeMath 依赖语言层 0.8 内置溢出回滚Math.sol 提供现代数学工具Pausable 的 owner 恶意破坏风险保留 pause/unpause 能力并配合时间锁类组件组合使用Pausable.sol测试不足仓库现已拥有覆盖各模块的完整测试套件见 test/ 目录与 形式化验证FV 体系需要说明的是2017 年被审计的CrowdsaleToken、Shareable、MultisigWallet、VestedToken、DayLimit等合约已不在当前仓库的 contracts/ 目录中——多签、众筹等功能要么被重构进更通用的模块如 AccessControl、TimelockController要么被移除当前仓库中也不再包含当年的 Truffle 测试。这些旧代码仅存在于历史提交中因此对它们的讨论完全基于审计报告本身。结论与启示从这份 2017 年的报告中可以提炼出几条至今仍然有效的工程原则原样部署是安全性的高门槛审计方对 CrowdsaleToken 的结论提醒我们任何把资金、信息或其他有价值资产交给链上代码的场景都不应部署未审计代码——这一点在报告开头便以一旦开发者改动 OpenZeppelin 合约代码就离开了已审计状态的方式被强调。检查-效果-交互Checks-Effects-Interactions是朴素且有效的思维框架MultisigWallet 的递归调用、PullPayment 的发送时序等问题的讨论本质上都在引导开发者把状态更新放在外部交互之前。错误处理风格需要全局一致throw vs return false 的争论最终以语言层面 revert 自定义错误 SafeERC20 兼容层收场今天的开发者应当遵循这一收敛后的惯例。审计的价值在于设计准则而非逐行挑刺报告多次呼吁统一风格、补充文档、明确设计意图——这些元层面的建议与具体 bug 同等重要。这份报告是 OpenZeppelin Contracts 安全文化的一个原点从审计方要求团队做得更好开始仓库在随后近十年里持续以公开审计报告audits/README.md 中 2018 年至今的历次记录为节点迭代最终成为今天以 revert 语义、结构化错误、内置溢出防护和形式化验证著称的现代智能合约库。读者在阅读现代源码时若能带着这份 2017 年报告的问题意识资金提取、重入、状态检查、错误一致性将更容易理解每一处设计决策背后的安全动因。【免费下载链接】openzeppelin-contractsOpenZeppelin Contracts is a library for secure smart contract development.项目地址: https://gitcode.com/GitHub_Trending/op/openzeppelin-contracts创作声明:本文部分内容由AI辅助生成(AIGC),仅供参考