SP1 Helios 代码审计报告

这是一份OpenZeppelin对SP1 Helios的代码审计报告,该报告详细分析了SP1 Helios代码中存在的安全问题、代码质量问题以及潜在的改进建议。报告发现了一个客户端报告的问题,可能导致无效的最终性更新,并提出了修复建议。此外,报告还指出了代码中存在的低危漏洞、拼写错误、文档缺失、冗余操作等问题。

目录

摘要

类型:跨链时间线 从 2025-04-02 到 2025-04-08 语言:Rust,Solidity 总问题:17(解决 16 个) 严重性问题:临界 0(0 个解决) 高 0(0 个解决) 中 0(0 个解决) 低 3(解决 2 个) 备注及其他信息 13(解决 13 个) 客户报告问题 1(解决 1 个)

范围

OpenZeppelin 对 SP1 Helios 库中在 pull request #2 中进行的更改进行了 diff 审计,提交为 6d554b9

审计范围包括以下文件:

 .
├── contracts/src
│   └── SP1Helios.sol
├── primitives/src
│   ├── lib.rs
│   └── types.rs
└── program/src
    └── main.rs

系统概述

SP1 Helios 是一个由 Succinct 开发的 ZK 轻客户端,将 Helios 嵌入到 SP1 ZK 虚拟机中。它用于验证目标链执行环境中源链的一致性。例如,SP1 Helios 可以在 L2 上运行以验证以太坊主网的一致性。这个过程生成一个 ZK 证明,证明轻客户端成功地将其状态更新到以太坊上由当前验证者集签名的最新最终状态。

在 SP1 内部执行时,Helios 接收以太坊的信标区块头,验证某个状态是否被包含,检查所有相关的一致性证明(例如 BLS 签名、同步委员会等),并更新其状态。SP1 生成此执行的简洁 ZK 证明,然后由目标链上的链下验证合约进行验证。因此,后者不需要直接验证以太坊的一致性。相反,它验证 SP1 证明,证明 Helios 正确运行并看到有效的以太坊数据。

审计的拉取请求通过向证明系统添加存储槽证明,修改了 Succinct 原始的 SP1 Helios 实现。此外,验证合约已更新以接受有效的 L1 证明,并随后在 L2 上存储证明的存储槽。此更新使得证明特定以太坊合约在特定区块编号的某个存储槽中持有特定值成为可能。

安全模型和信任假设

在审计期间,假定 Succinct 的 SP1 Helios 原始实现是安全和可靠的。此外,做出了以下信任假设。

SP1 Helios 轻客户端操作和更新角色

  • SP1 Helios 轻客户端将由 Risk Labs 操作。
  • SP1Helios 合约中的 update 函数限制为授权更新者:

    • 更新者集是不可变的—在部署时定义,之后不能更改,意味着在部署后不允许添加或删除新地址。
    • Risk Labs 将控制更新角色,确保只有受信任的实体可以提交更新。
  • SP1 Helios 客户端及 SP1Helios 合约将按照社区的最佳利益进行管理。

低严重性

SP1Helios 合约可能变为不可更新

SP1Helios 合约的 167 行 上的检查防止更新者在超过 1 周的时间内更新存储槽。如果更新者未能在较长时间内调用 update 函数,以致于最新有效的 fromHead 将至少有 1 周的历史,则 update 函数将始终反转,导致 SP1Helios 合约变得不可更新。

依赖 SP1Helios 合约数据的合约(例如预言机或桥接)可能在 SP1Helios 合约永久卡住后面临意外的反转。假设持续更新的系统需要依赖紧急重写或迁移,可能以不可预测的方式破坏可组合性。

考虑实施一个故障保护机制以防止永久性故障。或者,考虑记录 SP1Helios 依赖的风险,并概述可能的缓解步骤,以确保依赖系统能够优雅地恢复。

更新:承认,但未解决。团队表示:

_我们有意不将 SP1Helios 合约设置为可拥有合约,因为我们希望任何人都能“无信任”使用它。我们的架构设计是鼓励此合约的客户端(如 UniversalSpokePool)在此轻客户端变得故障时有故障保护。例如,UniversalSpokePool 在 这里 有注释,解释如果 Helios 合约变得故障该怎么办。这个 SpokePool 是可升级的,如果有必要,可以重新部署其实现 指向新的 Helios 轻客户端。SpokePool 通过存储所有已执行消息的 messageNonce 内置了重放保护,即使 Helios 合约重新部署也无法重放,假设 HubPoolStore 不需要升级,且 Helios 合约更新是安全且有效的 L1 状态根。_

角色管理员的多余更新

SP1Helios 合约继承了 AccessControlEnumerable 合约以启用基于角色的访问控制机制。根据 评论UPDATER_ROLE 应该是无管理员的,使得更新者集在部署后成为不可变的。如 AccessControl 合约中提到的那样,所有角色的默认管理员角色是 DEFAULT_ADMIN_ROLE ,这意味着只有具有该角色的帐户才能授予或撤销其他角色。

然而,DEFAULT_ADMIN_ROLE 的值被设置为 bytes32(0),这使得在 141 行 中将 UPDATER_ROLE 的管理员设置为 bytes32(0) 的调用是多余的。此外,SP1Helios.t.sol70 行 的测试中的注释表明 UPDATER_ROLE 应该是它自己的管理员,这与之前提到的 SP1Helios 中的注释相矛盾,即 UPDATER_ROLE 应该是无管理员的。此外,在 71 行 的后续检查仅检查 initialUpdater 是否具有 UPDATER_ROLE,而不是 initialUpdater 是否是 UPDATER_ROLE 的管理员。

考虑删除 SP1Helios 中的 _setRoleAdmin 调用,以确保 UPDATER_ROLE 没有管理员。由于没有地址被分配给 DEFAULT_ADMIN_ROLEUPDATER_ROLE 的管理员将仍然为 DEFAULT_ADMIN_ROLE,且该角色没有分配地址。此外,考虑修复测试中的注释,以使其与 SP1Helios 合约中的文档对齐。

更新:pull request #19 中已解决。

误导性的文档

在整个代码库中,识别出多个误导性文档的实例:

  • "3. 验证存储槽证明" 的注释放错了位置,应移至 verify_storage_slot_proofs 之上。此外,步骤编号 (3) 是错误的 - 根据 程序流程,应为 (4)。
  • 此注释 的步骤编号 (4) 应为 (6),根据 程序流程
  • main 中相关位置未标记为注释的 "3. 验证执行状态根证明" 和 "5. 断言所有更新有效" 步骤,与其他步骤 (1、2、4、6) 相对立。
  • storageValues 映射的注释假设它是从元组 (blockNumber, contract, slot) 映射而来的,而不是通过哈希这些值计算出键。
  • main.rs 中的 注释 提到 contract_trie_value_bytesvalue 变量。然而,这两个值不清楚指的是什么。考虑进一步澄清此注释。此外,在提到注释中的变量时,考虑将单引号替换为反引号。
  • SP1Helios 合约中 189 行199 行 的注释假设后续检查确保新头未被设置。然而,这些检查要么确保新头尚未设置,要么确保新头等于设置为新头的头部。

考虑纠正上述注释,以提高代码库的整体清晰度和可读性。

更新:pull request #12 中已解决。

备注及其他信息

排版错误

在整个代码库中,识别出多个排版错误的实例:

  • main.rs21 行 中,“Asset” 应该为 “Assert”。
  • SP1Helios.sol189 行199 行 中,“hasnt” 应该为 “hasn't”。
  • SP1Helios.sol221 行229 行 中,“peroid” 应该为 “period”。

考虑修复上述列出的排版错误,以提高代码库的整体清晰度和可读性。

更新:pull request #11 中已通过提交 31c682c 解决。

不必要的 unused_imports

build.rs1 行 中的 allow(unused_imports) 属性允许导入未使用的组件。然而,导入的两个组件 build_program_with_argsBuildArgs 实际上在 5 行7 行 中被使用。

考虑移除 allow(unused_imports) 属性,以增强代码清晰度并防止混淆。

更新:pull request #13 中已解决。

命名建议

在代码库中识别出多个改进命名的机会:

  • SlotBehindHead 错误 可以重命名为 SlotNotAfterHead,因为它在新头在之前的头之后或相等时被抛出。
  • SP1Helios 合约中 219 行period 变量可以重命名为 newPeriod

考虑重命名上述变量,以提高代码可读性。

更新:pull request #11 中已通过提交 31c682c 解决。团队表示:

恢复错误名称。更改为 NonIncreasingHeadSlotBehindHead 用于比较 po.newHead 和存储在合约中的 head,所以这是有意义的。我觉得 SlotNotAfterHead 会继承相同的语义。NonIncreasingHead 感觉更清晰。

不必要的克隆

main.rs124 行 中,clone 是不必要的,因为 rlp_encoded_trie_account 在代码后面未被使用。这使得将所有权传递给 verify_proof 函数 是安全的。

考虑避免不必要的克隆,以提高程序的效率。

更新:pull request #11 中已通过提交 e76912f 解决。

缺失的文档字符串

SP1Helios.sol 中识别出多个缺失文档字符串的实例:

考虑彻底记录所有事件(及其参数),这些事件作为任何合约的一部分。当编写文档字符串时,考虑遵循 以太坊自然规范格式(NatSpec)。

更新:pull request #12 中已解决。

重复的存储写入和事件发射

SP1Helios 合约的 190 行 中,当 headers[po.newHead] 不为空且与 po.newHeader 不同,检查会失败。然而,在 194 行 中,无论其值是否已经等于 po.newHeaderheaders[po.newHead] 都将被更新。

同样,在 200 行 中,当 executionStateRoots[po.newHead] 不为空且与 po.executionStateRoot 不同,检查会失败。然而,在 208 行 中,executionStateRoots[po.newHead] 的更新,即使它已等于 po.executionStateRoot ,也是如此。此外,在 209 行 中,当存储被更新时,总是会发射事件,甚至当值保持不变时。

考虑修改逻辑,仅在值实际变化时更新存储和发射事件。这将优化Gas使用,并通过避免冗余操作提高代码清晰度。

更新:pull request #14 中已通过提交 94123f5 解决。

映射中缺失的命名参数

Solidity 0.8.18 起,开发人员可以在映射中使用命名参数。这意味着映射可以采取 mapping(KeyType KeyName? => ValueType ValueName?) 的形式。这种更新的语法提供了对映射目的的更透明表示。

SP1Helios.sol 中识别出多个没有命名参数的映射实例:

考虑向映射添加命名参数,以提高代码库的可读性和可维护性。

更新:pull request #15 中已解决。

未使用的状态变量

SP1Helios.sol 中,GENESIS_VALIDATORS_ROOTSOURCE_CHAIN_ID 状态变量被初始化但未使用。

为改善代码库的整体清晰度和意图,考虑删除任何未使用的状态变量或在备注中明确说明其存在。

更新:pull request #21 中已解决。

缺少安全联系人

在智能合约中提供特定的安全联系人(例如电子邮件或 ENS 名称)显著简化了个人在识别代码中的漏洞时与其沟通的过程。这种做法非常有利,因为它允许代码所有者决定漏洞披露的沟通渠道,消除由于缺乏知识而导致的误沟通或未报告风险。此外,如果合约包含第三方库,并且出现错误,那么其维护者也可以更轻松地联系合适的人来解决问题并提供缓解指令。

SP1Helios 合约 没有安全联系人。

考虑在每个合约定义的上方添加一个包含安全联系人的 NatSpec 注释。建议使用 @custom:security-contact 约定,因为它已被 OpenZeppelin Wizard 以及 ethereum-lists 采用。

更新:pull request #12 中已解决。

函数可见性过于宽松

SP1Helios.sol 中,getCurrentEpochheadTimestamp public 函数可见性过于宽松。相反,它们的可见性可以限制为 external

为了更好地传达函数的预期用途,并可能实现一些额外的Gas节省,考虑将函数的可见性更改为仅限所需的权限。

更新:pull request #16 中已解决。

require 语句中的自定义错误

自 Solidity 版本 0.8.26 起,已向 require 语句添加了自定义错误支持。虽然最初此功能仅在 IR 管道中可用,但 Solidity 0.8.27 将支持扩展到了遗留管道。

SP1Helios.sol 中包含多个可以用 require 语句替换的 if-revert 语句:

为了简洁和节省Gas,考虑将 if-revert 语句替换为 require 语句。

更新:pull request #20 中已解决。

过时且未修复的 Solidity 版本

使用过时和未修复的 Solidity 版本可能导致合约中的漏洞和意外行为。

SP1Helios.sol 有一个 solidity ^0.8.22 的浮动 pragma 指令。Pragma 指令应该是固定的,以清楚地识别合同将编译的 Solidity 版本。此外,此 Solidity 版本是过时的。

考虑利用 最新的 Solidity 版本 以提高代码库的整体可读性和安全性。无论使用哪个版本的 Solidity,都应考虑固定版本以防止由于未来不兼容的发布而导致的错误。

更新:pull request #17 中已解决。

前缀递增运算符 (++i) 可以节省循环中的Gas

SP1Helios.sol 中,i++ 操作 [1] [2] 可以优化以节省Gas。

考虑在循环中使用前缀递增运算符 (++i) 而不是后缀递增运算符 (i++),以节省Gas。这种优化跳过了在增量操作之前存储值的步骤,因为表达式的返回值被忽略。

更新:pull request #18 中已解决。

客户报告

缺失检查可能允许无效最终性更新

main.rs 中的 main 函数 负责处理传入的证明输入,并生成可以稍后提交给 SP1Helios 合约的有效证明。证明生成过程包括多个验证步骤。这些步骤包括应用最终性更新和验证状态根证明的执行。

通过在 58 行 调用 verify_finality_update 函数来验证最终性更新。该函数检查与最终性 信标区块头 相关的执行负载是否包含在 SSZ Merkle trie 下,这个 trie 的根位于信标区块头的 body_root 字段中。执行负载本身包含执行状态根 (state_root),唯一标识以太坊全局状态。

该检查是通过验证从执行负载(作为 SSZ trie 中的一个叶子)到根 body_root 的 Merkle 路径来进行的,它有效地将执行负载和嵌套状态根绑定到信标头,从而确保共识层和执行层之间的一致性。

Helios 中的验证检查遵循以下函数调用序列:

  1. verify_finality_update 启动该过程以下调用
  2. verify_generic_update,然后调用
  3. is_valid_header 来验证头部结构,触发
  4. is_execution_payload_proof_valid 进行负载验证,最终使用
  5. is_proof_valid 来执行来自 header.execution() 的执行负载的最后 Merkle 证明,与存储在 header.beacon().body_rootbody_root 进行比较。

在验证最终性更新后,通过在 71 行 调用 apply_finality_update 将其应用于客户端 store。实际上,store 的更新发生在 apply_update_no_quorum_check 中,而该函数是否被执行由 should_apply_update 标志决定。具体来说,当 store 被更新(should_apply_update == true)的条件为:

  1. 有大多数 验证者签署了更新,并且
  2. 更新为比上次最终性更新的槽 严格更大 的槽,或这是一个用于最终化下一个同步委员会的 更新

假设两个条件均为 true,则调用 apply_update_no_quorum_check,其更新 store 的所有字段,除了 store.finalized_header(该头部也包含执行负载,如上所述)。后者只在 末尾 更新,但仅在 满足条件 时,即更新为严格大于存储的最后一次最终化更新的槽的槽(上面条件 2 的第一部分)。

因此,如果我们有一个最终性更新,该槽等于存储中的最后一次最终性更新的槽(即 update_finalized_slot == store.finalized_header.beacon().slot这个上下文中),但对于该更新,拥有大多数(从而满足条件 1),并且这是一个用于最终化下一个同步委员会的(从而满足条件 2)的更新,则将调用 apply_update_no_quorum_check,更新 store 的所有字段,除了 store.finalized_header。后者将不会更新,因为 此条件 由于更新和存储中的最后一个最终块的槽号相同而评估为 false。因此,存储将更新为具有无效执行负载的头,从而破坏共识层和执行层之间的一致性。

重要的是,SP1Helios 验证合约中未防止上述场景。确实,验证者 确保 更新的槽(po.newHead)严格大于存储中的最后一次最终化槽(fromHead),但未声明后者必须与 po.prevHead 相同,在客户端执行过程中实际提交的最后最终化槽。因此,该合约允许将 fromHead < po.newHead 作为对 update 函数的 输入,以便在验证器中通过 此检查,而将 po.prevHead 设置为 po.newHead 使得在客户端执行中 这个条件 失败。

考虑通过将 fromHeadupdate 函数的 第三输入 中移除并在函数内设置 fromHead = po.prevHead 来正确绑定 fromHeadpo.prevHead。此外,在客户端执行方的 apply_finality_update 之后 需要添加检查,以确保客户端更新到的最终区块的槽严格大于初始槽 prev_head。这将确保 apply_update_no_quorum_check 中的 if 子句 已满足,并且执行负载已在存储中正确更新。

更新:pull request #11 中已通过提交 31c682c 解决。

结论

Across Protocol 在 PR# 2 中对 SP1 Helios ZK 以太坊轻客户端引入的更改通过将存储槽值的证明作为 SP1 证明的一部分添加了新功能。这有效地允许客户端生成 L1 合约的特定存储槽值的证明,并将它们存储在 L2 上的 SP1Helios 合约中。这是在 Across Protocol 的跨链桥接上下文中所需的特性。

报告描述了一个客户端报告的问题,在特定条件下可能允许无效的最终性更新。这个漏洞使得客户端能够成功应用存储了无效执行负载的最终性更新,可能导致意想不到的后果。问题的根源在于代码库内外部之间的不一致。除此之外,实施被认为是可靠的,基于所述的信任假设。报告中提到三个低严重性问题,以及旨在提高代码质量和文档的若干建议。我们对 Across 团队在整个审计过程中的响应能力表示赞赏。

  • 原文链接: blog.openzeppelin.com/sp...
  • 登链社区 AI 助手,为大家转译优秀英文文章,如有翻译不通的地方,还请包涵~
点赞 0
收藏 1
分享
本文参与登链社区写作激励计划 ,好文好收益,欢迎正在阅读的你也加入。

0 条评论

请先 登录 后评论
OpenZeppelin
OpenZeppelin
江湖只有他的大名,没有他的介绍。