这是一份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 原始实现是安全和可靠的。此外,做出了以下信任假设。
SP1Helios
合约中的 update
函数限制为授权更新者:
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.sol
中 70 行 的测试中的注释表明 UPDATER_ROLE
应该是它自己的管理员,这与之前提到的 SP1Helios
中的注释相矛盾,即 UPDATER_ROLE
应该是无管理员的。此外,在 71 行 的后续检查仅检查 initialUpdater
是否具有 UPDATER_ROLE
,而不是 initialUpdater
是否是 UPDATER_ROLE
的管理员。
考虑删除 SP1Helios
中的 _setRoleAdmin
调用,以确保 UPDATER_ROLE
没有管理员。由于没有地址被分配给 DEFAULT_ADMIN_ROLE
,UPDATER_ROLE
的管理员将仍然为 DEFAULT_ADMIN_ROLE
,且该角色没有分配地址。此外,考虑修复测试中的注释,以使其与 SP1Helios
合约中的文档对齐。
更新:在 pull request #19 中已解决。
在整个代码库中,识别出多个误导性文档的实例:
storageValues
映射的注释假设它是从元组 (blockNumber
, contract
, slot
) 映射而来的,而不是通过哈希这些值计算出键。main.rs
中的 注释 提到 contract_trie_value_bytes
和 value
变量。然而,这两个值不清楚指的是什么。考虑进一步澄清此注释。此外,在提到注释中的变量时,考虑将单引号替换为反引号。SP1Helios
合约中 189 行 和 199 行 的注释假设后续检查确保新头未被设置。然而,这些检查要么确保新头尚未设置,要么确保新头等于设置为新头的头部。考虑纠正上述注释,以提高代码库的整体清晰度和可读性。
更新:在 pull request #12 中已解决。
在整个代码库中,识别出多个排版错误的实例:
main.rs
的 21 行 中,“Asset” 应该为 “Assert”。SP1Helios.sol
的 189 行 和 199 行 中,“hasnt” 应该为 “hasn't”。SP1Helios.sol
的 221 行 和 229 行 中,“peroid” 应该为 “period”。考虑修复上述列出的排版错误,以提高代码库的整体清晰度和可读性。
更新:在 pull request #11 中已通过提交 31c682c 解决。
在 build.rs
的 1 行 中的 allow(unused_imports)
属性允许导入未使用的组件。然而,导入的两个组件 build_program_with_args
和 BuildArgs
实际上在 5 行 和 7 行 中被使用。
考虑移除 allow(unused_imports)
属性,以增强代码清晰度并防止混淆。
更新:在 pull request #13 中已解决。
在代码库中识别出多个改进命名的机会:
SlotBehindHead
错误 可以重命名为 SlotNotAfterHead
,因为它在新头在之前的头之后或相等时被抛出。SP1Helios
合约中 219 行 的 period
变量可以重命名为 newPeriod
。考虑重命名上述变量,以提高代码可读性。
更新:在 pull request #11 中已通过提交 31c682c 解决。团队表示:
恢复错误名称。更改为
NonIncreasingHead
。SlotBehindHead
用于比较po.newHead
和存储在合约中的head
,所以这是有意义的。我觉得SlotNotAfterHead
会继承相同的语义。NonIncreasingHead
感觉更清晰。
在 main.rs
的 124 行 中,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.newHeader
,headers[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_ROOT
和 SOURCE_CHAIN_ID
状态变量被初始化但未使用。
为改善代码库的整体清晰度和意图,考虑删除任何未使用的状态变量或在备注中明确说明其存在。
更新:在 pull request #21 中已解决。
在智能合约中提供特定的安全联系人(例如电子邮件或 ENS 名称)显著简化了个人在识别代码中的漏洞时与其沟通的过程。这种做法非常有利,因为它允许代码所有者决定漏洞披露的沟通渠道,消除由于缺乏知识而导致的误沟通或未报告风险。此外,如果合约包含第三方库,并且出现错误,那么其维护者也可以更轻松地联系合适的人来解决问题并提供缓解指令。
SP1Helios
合约 没有安全联系人。
考虑在每个合约定义的上方添加一个包含安全联系人的 NatSpec 注释。建议使用 @custom:security-contact
约定,因为它已被 OpenZeppelin Wizard 以及 ethereum-lists 采用。
更新:在 pull request #12 中已解决。
在 SP1Helios.sol
中,getCurrentEpoch
和 headTimestamp
public
函数可见性过于宽松。相反,它们的可见性可以限制为 external
。
为了更好地传达函数的预期用途,并可能实现一些额外的Gas节省,考虑将函数的可见性更改为仅限所需的权限。
更新:在 pull request #16 中已解决。
自 Solidity 版本 0.8.26
起,已向 require
语句添加了自定义错误支持。虽然最初此功能仅在 IR 管道中可用,但 Solidity 0.8.27
将支持扩展到了遗留管道。
SP1Helios.sol
中包含多个可以用 require
语句替换的 if-revert
语句:
if (params.updaters.length == 0) {\\ revert NoUpdatersProvided();\\ }
语句if (headers[fromHead] == bytes32(0)) {\\ revert PreviousHeadNotSet(fromHead);\\ }
语句if (block.timestamp - slotTimestamp(fromHead) > MAX_SLOT_AGE) {\\ revert PreviousHeadTooOld(fromHead);\\ }
语句if (po.newHead <= fromHead) {\\ revert SlotBehindHead(po.newHead);\\ }
语句if (currentSyncCommitteeHash != po.startSyncCommitteeHash) {\\ revert SyncCommitteeStartMismatch(po.startSyncCommitteeHash, currentSyncCommitteeHash);\\ }
语句if (headers[po.newHead] != bytes32(0) && headers[po.newHead] != po.newHeader) {\\ revert InvalidHeaderRoot(po.newHead);\\ }
语句if (\\ executionStateRoots[po.newHead] != bytes32(0)\\ && executionStateRoots[po.newHead] != po.executionStateRoot\\ ) {\\ revert InvalidStateRoot(po.newHead);\\ }
语句if (syncCommittees[nextPeriod] != bytes32(0)) {\\ revert SyncCommitteeAlreadySet(nextPeriod);\\ }
语句为了简洁和节省Gas,考虑将 if-revert
语句替换为 require
语句。
更新:在 pull request #20 中已解决。
使用过时和未修复的 Solidity 版本可能导致合约中的漏洞和意外行为。
SP1Helios.sol
有一个 solidity ^0.8.22 的浮动 pragma 指令。Pragma 指令应该是固定的,以清楚地识别合同将编译的 Solidity 版本。此外,此 Solidity 版本是过时的。
考虑利用 最新的 Solidity 版本 以提高代码库的整体可读性和安全性。无论使用哪个版本的 Solidity,都应考虑固定版本以防止由于未来不兼容的发布而导致的错误。
更新:在 pull request #17 中已解决。
在 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 中的验证检查遵循以下函数调用序列:
verify_finality_update
启动该过程以下调用verify_generic_update
,然后调用is_valid_header
来验证头部结构,触发is_execution_payload_proof_valid
进行负载验证,最终使用is_proof_valid
来执行来自 header.execution()
的执行负载的最后 Merkle 证明,与存储在 header.beacon().body_root
的 body_root
进行比较。在验证最终性更新后,通过在 71 行 调用 apply_finality_update
将其应用于客户端 store
。实际上,store
的更新发生在 apply_update_no_quorum_check
中,而该函数是否被执行由 should_apply_update
标志决定。具体来说,当 store
被更新(should_apply_update == true
)的条件为:
假设两个条件均为 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
使得在客户端执行中 这个条件 失败。
考虑通过将 fromHead
从 update
函数的 第三输入 中移除并在函数内设置 fromHead = po.prevHead
来正确绑定 fromHead
到 po.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 助手,为大家转译优秀英文文章,如有翻译不通的地方,还请包涵~
如果觉得我的文章对您有用,请随意打赏。你的支持将鼓励我继续创作!