ZKsync V29 版本发布审计

本次审计主要关注了matter-labs/era-contracts仓库,重点审查了“减少互操作性”和“快速最终确定性”两个新特性。审计范围包括L1和系统合约,以及对代码质量、潜在安全风险和gas优化等方面的分析,发现了包括高危漏洞在内的多个问题,并提供了相应的改进建议。审计团队与Matter Labs团队保持了积极沟通,问题已在后续提交中得到解决。

目录

总结

TypeLayer 2 & RollupsTimeline从 2025-05-20 到 2025-06-26 编程语言SolidityYul问题总数22(22个已解决)严重程度高的问题0(0个已解决)严重程度高的问题3(3个已解决)严重程度中的问题2(2个已解决)严重程度低的问题8 (8 已解决) 说明和附加信息 8 (8 已解决)

范围

OpenZeppelin 对 matter-labs/era-contracts 仓库针对基础提交 7278847 进行了差异审计。审计分两个阶段进行:

第一阶段:在 head commit 3092fc0 处减少互操作性功能。

以下文件在范围内:

 l1-contracts
  ├── contracts
  │   ├── bridgehub
  │   │   ├── IMessageRoot.sol
  │   │   ├── L2MessageVerification.sol
  │   │   └── MessageRoot.sol
  │   ├── common
  │   │   ├── Messaging.sol
  │   │   ├── interfaces
  │   │   │   └── IL2InteropRootStorage.sol
  │   │   └── libraries
  │   │       └── MessageHashing.sol
  │   └── state-transition
  │       ├── chain-deps
  │       │   └── facets
  │       │       ├── Executor.sol
  │       │       ├── Mailbox.sol
  │       │       └── MessageVerification.sol
  │       └── chain-interfaces
  │           ├── IExecutor.sol
  │           ├── IMailbox.sol
  │           ├── IMailboxImpl.sol
  │           └── IMessageVerification.sol
  system-contracts
  ├── bootloader
  │   └── bootloader.yul
  └── contracts
      └── L2InteropRootStorage.sol

第二阶段:在 head commit 903cfed2 处的快速终结性功能和系统中几个部分的更小变化。

以下文件在范围内:

 l1-contracts
  ├── contracts
  │   ├── bridge
  │   │   └── L1BridgeContractErrors.sol
  │   ├── bridgehub
  │   │   ├── Bridgehub.sol
  │   │   ├── CTMDeploymentTracker.sol
  │   │   ├── ChainAssetHandler.sol
  │   │   ├── IBridgehub.sol
  │   │   ├── IChainAssetHandler.sol
  │   │   └── L1BridgehubErrors.sol
  │   ├── chain-registrar
  │   │   └── ChainRegistrar.sol
  │   ├── common
  │   │   ├── Config.sol
  │   │   ├── L1ContractErrors.sol
  │   │   ├── Messaging.sol
  │   │   ├── interfaces
  │   │   │   └── IL1Messenger.sol
  │   │   ├── l2-helpers
  │   │   │   ├── IL2ToL1Messenger.sol
  │   │   │   ├── L2ContractAddresses.sol
  │   │   │   ├── L2ContractHelper.sol
  │   │   │   └── SystemContractsCaller.sol
  │   │   └── libraries
  │   │       └── MessageHashing.sol
  │   ├── governance
  │   │   ├── PermanentRestriction.sol
  │   │   └── ServerNotifier.sol
  │   ├── state-transition
  │   │   ├── AccessControlEnumerablePerChainAddressUpgradeable.sol
  │   │   ├── ChainTypeManager.sol
  │   │   ├── IChainTypeManager.sol
  │   │   ├── IValidatorTimelock.sol
  │   │   ├── L1StateTransitionErrors.sol
  │   │   ├── ValidatorTimelock.sol
  │   │   ├── chain-deps
  │   │   │   ├── DiamondInit.sol
  │   │   │   ├── GatewayCTMDeployer.sol
  │   │   │   ├── ZKChainStorage.sol
  │   │   │   └── facets
  │   │   │       ├── Admin.sol
  │   │   │       ├── Executor.sol
  │   │   │       └── ZKChainBase.sol
  │   │   ├── chain-interfaces
  │   │   │   ├── IAdmin.sol
  │   │   │   ├── IDiamondInit.sol
  │   │   │   └── IExecutor.sol
  │   │   ├── l2-deps
  │   │   │   └── IL2GenesisUpgrade.sol
  │   │   ├── libraries
  │   │   │   └── BatchDecoder.sol
  │   │   └── verifiers
  │   │       ├── L1VerifierFflonk.sol
  │   │       └── L2VerifierPlonk.sol
  │   ├── transactionFilterer
  │   │   └── GatewayTransactionFilterer.sol
  │   └── upgrades
  │       ├── BaseZkSyncUpgrade.sol
  │       ├── BaseZkSyncUpgradeGenesis.sol
  │   │   ├── IL2V29Upgrade.sol
  │   │   ├── L1V29Upgrade.sol
  │   │   ├── UpgradeStageValidator.sol
  │   │   └── ZkSyncUpgradeErrors.sol
  system-contracts
  ├── bootloader
  │   └── bootloader.yul
  ├── contracts
  │   ├── Constants.sol
  │   ├── EvmEmulator.yul
  │   ├── EvmPredeploysManager.sol
  │   ├── L2GenesisForceDeploymentsHelper.sol
  │   ├── L2InteropRootStorage.sol
  │   ├── L2UpgradeUtils.sol
  │   ├── L2V29Upgrade.sol
  │   ├── PubdataChunkPublisher.sol
  │   ├── SystemContext.sol
  │   ├── SystemContractErrors.sol
  │   ├── interfaces
  │   │   ├── IBridgedStandardERC20.sol
  │   │   ├── IBridgehub.sol
  │   │   ├── IL2AssetRouter.sol
  │   │   ├── IL2GenesisUpgrade.sol
  │   │   ├── IL2NativeTokenVault.sol
  │   │   └── IPubdataChunkPublisher.sol
  │   └── precompiles
  │       ├── CodeOracle.yul
  │   │   ├── EcAdd.yul
  │   │   ├── EcMul.yul
  │   │   └── Modexp.yul
  └── evm-emulator
      ├── EvmEmulatorLoop.template.yul
      └── EvmEmulatorLoopUnusedOpcodes.template.yul

更新:在修复审查期间,还调查了 pull requests #1538#1540。所有报告的问题都已在 commit babbed5 中完全解决。

系统概述

降低互操作性功能

到目前为止,L2 ZKChains 只能与其结算层进行通信。降低互操作性功能支持在 Gateway 上结算的 ZKChains 之间进行跨链纯消息传递。已开发的机制为未来发布完全互操作性功能奠定了基础,届时将支持 L2 ZKChains 之间的实际价值转移。

降低互操作性可以看作是一种自动化机制,其中 Gateway 上已结算的批次的承诺会共享并存储在所有 ZKChains 上。因此,为了在 ZKChain B 上证明特定交易发生在 ZKChain A 上,用户应提交针对包含该交易的已结算批次的承诺的有效交易日志包含的 Merkle 证明。

下面给出了新机制基本流程的分步描述:

  • 每次 ZKChain 在 Gateway 上结算时,其相应的 chainTree 都会通过附加新的批次叶子进行更新。这会导致新的 chainRoot,因此 sharedTree 中相应的叶子发生变化,从而导致更新的 sharedTree 根。
  • 每次密封包含批次结算的 Gateway 批次时,上次更新的 sharedTree 根会反映该 Gateway 批次中所有已结算的 ZKChain 批次。
  • Gateway 批次中上次更新的 sharedTree是一个 interop 根。外部节点会监视网络中的 NewInteropRoot 事件,并将新的 interop 根广播到所有在 Gateway 上结算的 ZKChains。
  • 在运行新的启动加载器实例时,每个 ZKChain 的操作员负责用新广播的 interop 根填充启动加载器内存
  • 随着启动加载器的运行,它遍历 interop 根并将它们存储在 ZKChain 的 L2InteropRootStorage 合约中。
  • 当启动加载器实例完成执行时,会准备一个相应的 ZKChain 批次以进行结算。该批次还包含有关存储的 interop 根的信息
  • 当该批次在 Gateway 上结算时,会检查存储在 ZKChain 上的所有 interop 根都是 Gateway 上 sharedTree 的有效根。否则,该批次将被回滚。
  • 批次成功结算后,可以在 ZKChain 上证明 interop 根下的日志包含。

快速终结性功能

快速终结性功能使在 Gateway 上结算的 ZKChains 能够更快地实现终结时间。节点可以监视 L1 上的预提交,而不是等待 Gateway 批次承诺、证明和执行(这对于操作员来说也是 gas 密集型的,无法频繁执行)。这使第三方服务(例如支持 ZKChains 的交易所)能够提供更快的结算,从而改善用户体验。

在链上,这是通过在 L1 上“预提交”在 Gateway 上处理的交易的排序、内容和执行状态来实现的。具体来说,每个交易哈希与其执行状态一起附加到滚动哈希,并在预提交期间存储在 L1 上。然后,针对在启动加载器中计算的 Gateway 交易的滚动哈希验证此滚动哈希,在批次承诺期间,如果两者不匹配,则回滚承诺。实际上,这意味着这些预提交的消费者(例如,交易所)可以依靠这些 Gateway 交易在批次中稍后以特定顺序和特定状态执行。否则,链将停滞。由于 Gateway 交易包含 ZKChain 批次的承诺、证明和执行,因此消费者可以推断这些 ZKChain 的状态。

此外,回滚 Gateway 上已承诺的批次和预提交的权利将从操作员转移到安全委员会。这意味着预提交只能由安全委员会回滚。更一般地说,通过 ValidatorTimelock 合约引入了一种灵活的机制来管理不同的角色(预提交、承诺、证明、执行和回滚)。

安全模型和信任假设

如前所述,互操作功能目前依赖于 ZKChain 操作员包含新的 interop 根。但是,应注意的是,由于树是仅附加的,因此只能通过操作员拒绝同步任何新根来审查消息。关于预提交,消费者必须了解它们是什么以及提供什么保证。

预提交保证了 Gateway 交易的顺序和执行状态。虽然此保证功能强大,因为预计这些交易是确定性的,但 Gateway 操作员仍然存在非确定性的来源,因为它能够影响诸如块时间戳、块号、块哈希或包含 interop 根之类的变量。因此,预提交的消费者务必确保所有预提交的交易都是确定性的,即使在对抗性操作员的最坏情况下也是如此。例如,不应依赖于执行智能合约(仅当 block.timestamp 为偶数时才执行 ZKChain 批次)的交易,因为以后可以操纵时间戳以履行承诺,而无需执行该批次。通常,建议预提交的消费者确保所有预提交的交易都由 EOA 发送到白名单合约(例如,Executor.sol),以减轻此类情况发生的风险。

请注意,操作员始终可以提交随机数据作为预提交的一部分,这将迫使安全委员会进行干预以回滚这些数据。因此,应始终验证所有数据以对应于有效的交易。此外,预提交可能会因链升级而失效。

在审计期间做出了以下信任假设:

  • 相关合约的所有者和 ValidatorTimelock 合约 的特权参与者将诚实地行事。
  • 安全理事会将以协议及其用户的最大利益行事。
  • 系统升级将正确执行。

高危漏洞

缺乏对 bootloader 初始化值的验证

启动加载器使用多个变量来跟踪其在处理 interop 根时的进度:

  • LAST_PROCESSED_BLOCK_NUMBER
  • CURRENT_NUMBER_OF_ROOTS_IN_BLOCK
  • CURRENT_INTEROP_ROOT

但是,未验证这些变量是否初始化为 0。这种缺乏验证使得操作员可能以各种方式偏离启动加载器的预期操作。例如,操作员可以将 LAST_PROCESSED_BLOCK_NUMBERCURRENT_NUMBER_OF_ROOTS_IN_BLOCK 设置为非零值以忽略某些根,或者将 CURRENT_INTEROP_ROOT 设置为非常高的值以创建缓冲区溢出并从数组边界之外读取根。此类攻击可能导致一系列结果,包括伪造不正确的 interop 根,这可能导致用户资金被操作员窃取。

考虑确保在启动加载器执行期间正确初始化这些变量。

更新:已在 commit 14571a2 中通过 pull request #1505 解决。所有变量都显式初始化为 0。

操作员可以伪造任意 Interop 根

可以通过调用 L1Messenger 合约的 sendToL1 函数在 ZKChains 之间发送消息。此类消息在 L2_TO_L1_MERKLE_TREE 中聚合,并在 bootloader 在批次上执行结束时发送到结算层。然后,可以通过 ZKChain 的操作员在批次上执行 bootloader 时确保共享同一结算层的不同 ZKChain 传来的 Merkle 树的同步。操作员可以将 interop 根添加到批次的数据中,以便将它们存储在 ZKChain 上的 L2InteropRootStorage 合约 中。

为了确保同步有效的 interop 根,在结算期间,将存储的根与存储在结算层上的根进行比较。然后,可以通过提供针对 interop 根的 Merkle 证明来在目标 ZKChain 上证明跨链消息。但是,启动加载器的内存管理设计中存在缺陷,使得操作员可以绕过 interop 根验证机制。具体来说,在 bootloader 合约中,设置 interop 根的循环验证它们的循环可以迭代 INTEROP_ROOTS 数组的不同切片。

例如,恶意操作员可以传递一个 sides 长度为 0 的 interop 根作为第一个 interop 根,并将 CURRENT_INTEROP_ROOT 初始化为 1。然后,setInteropRootForBlock 函数将迭代索引 [1:100] 处的根,而验证循环将循环索引 [0] 并提前退出。结算将成功完成,因为 dependencyRootsRollingHash 为 0,就好像没有添加任何新根一样。此设计缺陷允许操作员伪造任意根。

问题的核心是设置和验证 interop 根的循环的域可能不同。为了解决这个问题,请考虑显式地将上述循环合并为单个循环,以确保添加的任何 interop 根始终得到验证。

更新:已在 commit 2aae32a 中通过 pull request #1521 解决。现在在 L2 上设置 interop 根时计算在结算期间验证的滚动哈希。

bootloader::setInteropRootForBlockinteropRoots 的索引不一致

bootloader 合约中,特别是在 setInteropRootForBlock 函数中,应用于 interopRoots 的索引方法存在不一致。getNumberOfInteropRootInCurrentBlock 函数旨在返回给定块的 interopRoots 的总数。此值似乎表示块中包含的 interopRoots 的计数,而没有任何参考在整个批次中处理的根的顺序。

相反,nextInteropRootNumber 用作一个索引,该索引考虑了整个批次中 interopRoots 的顺序。索引方法上的这种差异——一个值表示单个块中的绝对计数,另一个值表示整个批次中的顺序索引——导致了根本的不一致。由于它们不同的上下文基础,直接for 循环条件中比较这两个值是不合逻辑的。这种不一致可能导致仅存储L2InteropRootStorage 合约中部分 interopRoots 集合。

为了解决这个问题并确保处理 interopRoots 的清晰性和正确性,请考虑标准化索引方法以始终反映块中的绝对计数或批次中的顺序索引。

更新:已在 commit 4852fe8 中通过 pull request #1501 解决。现在循环迭代正确的范围。

中危漏洞

来自 bootloader 中重复信息的不一致风险

在处理 interop 根时,引导加载程序负责迭代存储在其内存中的几个变量。在此过程中,引导加载程序会遍历每笔交易,获取关联的区块号,并检索存储在 INTEROP_BLOCKS 中的根的数量。然后,引导加载程序 INTEROP_ROOTS 数组中读取那么多元素,该数组包含每个根的多个字段:已处理的区块、链 ID、依赖区块、sides 长度和 sides 数据。

但是,当处理根时,与当前根关联的“已处理区块”会与当前区块进行比较,如果已处理区块高于当前区块,则循环会提前中断。这种情况永远不应该发生,因为它表明在 INTEROP_BLOCKS 中设置了不正确的根数量。这可能允许操作员设置一个非常高的已处理区块值并忽略一些根。

从更高层次上讲,此问题是由于区块中的根数量和与每个根关联的区块之间的信息重复造成的。因此,请考虑删除重复信息,只保留根的数量,或者在遇到不正确的区块号时回滚。

更新:已在提交 518305epull request #1523 中解决。如果遇到区块号不一致的根,引导加载程序现在将回滚。

由于入口点不一致,跨链消息验证中可能存在的安全漏洞

MessageVerification 合约旨在促进来自 ZKChains 的消息或日志的验证,从而增强跨链通信能力。此合约由结算层上的 Mailbox facet 和 ZKChains 上的 L2MessageVerification 合约扩展。Mailbox facet 已经封装了结算层上消息验证的必要功能,使得 MessageVerification 的证明函数在这种上下文中变得多余。

当由于处理发送 ZKChain 的 chainId 时的差异,调用了 MessageVerification 中的证明函数而不是结算层上的 Mailbox 中的证明函数时,冗余就会变得有问题。Mailbox 在内部确定 chainId,从而确保消息验证的一致性和安全性。相反,MessageVerification 需要 chainId 作为外部参数,从而为不一致和不正确的参数提交打开了大门。这种差异削弱了跨链消息验证的安全性和可靠性,如下面的场景所示:

场景 1

来自 ZKChainA 的消息在其 Gateway 上的 Diamond 代理中得到正确证明,但使用的是 ZKChainB 的 chainId。尽管 chainId 不正确,但由于准确的证明数据和对错误 chainId 的不依赖,证明仍然成功。

场景 2

在 ZKChainB 的 Diamond 代理中证明来自 L1 上的 ZKChainA 的消息也会导致证明成功。这是因为 ZKChainA 的 chainId 是外部提供的,并且证明数据是准确的,最终的批处理根哈希验证发生在 Gateway 的 Diamond 代理上,该代理不区分在其上结算的 ZKChain。

考虑引入额外的验证逻辑来检测和拒绝提交的不一致参数的证明,尤其要注意可能滥用 chainId 的情况。具体来说,考虑始终验证用于证明消息的 ZKChain 的 chainId 是否是根据上下文预期的 chainId

更新:已在提交 6f9abb0pull request #1525 中解决。用于消息验证的 shared 函数在 MessageVerification 中被标记为 virtual,并被在 Mailbox 中验证 chainId 的函数覆盖。

低风险

合约变量和事件中的命名具有误导性

bootloaderMessageRoot 合约中,某些名称不能准确反映它们代表的数据,从而导致潜在的混淆。

bootloader.yul 中,CURRENT_NUMBER_OF_ROOTS_IN_BLOCK_SLOTCURRENT_NUMBER_OF_ROOTS_IN_BLOCK_BYTE 变量具有误导性,暗示它们代表根的数量,而实际上它们跟踪的是已处理的区块的数量。变量名与其真实目的之间的这种差异可能会导致对合约功能的误解。同样,INTEROP_BLOCKS_BEGIN_SLOTINTEROP_BLOCKS_BEGIN_BYTE 变量实际上计算的是每个区块的根的数量。除此之外,可以将 interopRootStartSlot 变量重命名为 interopRootStartByte,以保持 slot 与 byte 命名的连贯性。

MessageRoot.sol 中,Preimage 事件 在生成新的 chainTree 根(chainRoot)以及相应地更新其在 sharedTree 中的叶子时触发,该事件包括两个名为 chainRootpreimage 的参数。但是,命名不准确地暗示了两个参数之间存在直接的密码学关系,但事实并非如此。此外,事件名称及其参数没有传达理解更新所需的完整上下文,因为 sharedTree 叶的哈希需要 chainRoot 和关联链的 chainId 才能完全清晰。

对于 bootloader.yul,请考虑重命名已识别的变量以更准确地反映其目的。对于 MessageRoot.sol,请考虑重命名 Preimage 事件以更好地描述其含义及其参数之间的关系。这些更改旨在提高代码的可读性和清晰度。

更新:已在提交 7303766pull request #1506 中解决。所有提出的要点都已得到解决。

INTEROP_ROOTS 读取容易受到缓冲区溢出的影响

当设置新的 interop 根时,引导加载程序被设计为从索引 INTEROP_ROOTS 数组中的索引 CURRENT_INTEROP_ROOT 读取数据到索引 CURRENT_INTEROP_ROOT + INTEROP_BLOCKS[CURRENT_NUMBER_OF_ROOTS_IN_BLOCK] - 2。但是,INTEROP_ROOTS 数组最多可以容纳 100 个根。这表明这些内存读取容易受到缓冲区溢出的影响。例如,如果 CURRENT_INTEROP_ROOTINTEROP_BLOCKS[i] 太大,引导加载程序将从 INTEROP_ROOTS 数组之外的内存中读取,该区域存储 COMPRESSED_BYTECODES

虽然这种溢出的影响尚不清楚,但请考虑确保引导加载程序不会从数组边界之外读取根,以防止任何潜在问题。

更新:已在提交 13140a1pull request #1507 中解决。现在代码会在尝试从 interop 根内存缓冲区之外读取时回滚。

未使用和冗余的代码

这段未使用的代码增加了代码库的复杂性,并带来了维护开销,可能会模糊合约的逻辑和功能。

在整个代码库中,发现了多个未使用和冗余代码的实例:

  • bootloader.yul 中,setInteropRootForBlock 函数interopRootStartSlot 变量的初始赋值 从未在后续操作中使用。
  • BridgeHub.sol 中,与升级期间暂停功能相关的代码已过时,因为新的 ChainAssetHandler 负责它。考虑弃用关联的状态变量并删除关联的modifier函数
  • 尽管从优先级队列过渡到优先级树结构,但在 Diamond 代理 facet(Admin.solMailbox.solGetters.solZKChainBase.sol)中仍然存在对过时的优先级队列的多个引用。这些引用具有误导性,并且不能反映当前的架构,可能会导致混淆和错误,从而影响理解或扩展代码库。

考虑删除所有不必要的代码实例,以增强代码的清晰度和可维护性。

更新:已在 pull request #1514 中解决。Matter Labs 团队表示:

我们将在下一个版本中弃用 BrigdeHub 中的升级功能,因为我们仍然需要在升级之前暂停迁移,执行升级,然后取消暂停。

不正确的 INTEROP_ROOT_SLOT_SIZE 返回值

bootloader 合约中,每个 interopRoot 的存储被设计为占用 5 个内存槽,其中包括用于处理 interopRoot 的区块号、链 ID、依赖区块、sides 长度和 sides。但是,INTEROP_ROOT_SLOT_SIZE 函数错误地返回了值 6 而不是 5。

这种差异目前不会影响系统的运行,因为额外未使用的内存槽不会损害内存的一致性,并且基本上会被忽略。但是,预期槽大小分配与实际槽大小分配之间的这种不一致可能会导致不必要的内存碎片。更重要的是,它为未来的开发工作引入了一个潜在的错误来源,如果对槽大小的假设不正确,可能会导致更严重的问题。

考虑修复 INTEROP_ROOT_SLOT_SIZE 函数以返回 interopRoot 实际占用的准确槽数。此调整将有助于提高代码的清晰度和可维护性,从而降低后续开发阶段出现错误的风险。

更新:已在提交 abacb3cpull request #1521 中解决。

addInteropRoot 函数参数类型不一致

L2InteropRootStorage 合约的 addInteropRoot 函数旨在接受第三个参数作为 bytes32 数组。但是,在其对应的接口定义中,第三个参数被指定为单个 bytes32 值而不是数组。虽然此接口当前未在现有代码库中使用,但这种不匹配的存在引入了潜在的风险。合约实现与其接口定义之间的这种不一致本质上容易出错,并且可能导致未来开发或更新中的集成问题或失败。

为了降低未来错误的风险并提高代码的可维护性和清晰度,请考虑对齐合约实现及其接口中 addInteropRoot 函数的参数类型。

更新:已在提交 271380dpull request #1509 中解决。未使用的函数已从接口中删除。

零叶可以在 ZKChains 上进行验证

ZKChains 上跨链消息的验证由 L2MessageVerification 合约管理。但是,_proveL2LeafInclusion 函数允许在验证叶子时正确的批处理根为零。这允许通过提供 finalProofNode = true 的空日志叶子证明来验证叶子(bytes32(0))是在不存在的批处理号中发送的。

考虑在将 correctBatchRoot 与重建的根进行比较时,验证 correctBatchRoot 是否为非零值。

更新:已在提交 25c3046pull request #1510 中解决。

bootloader 结束时调用额外的 setInteropRootForBlock

在批处理中引导加载程序执行结束时,会调用 setInteropRootForBlock(MAXIMUM_L2_BLOCK_NUMBER()) 函数。此调用被记录为“设置所有剩余的 interop 根,即使是虚拟区块中的根”。但是,在与 Matter Labs 团队讨论后,明确表示虚拟区块中不应存在 interop 根。

考虑删除上述代码以减少攻击面。

更新:已在提交 73be8f2pull request #1515 中解决。

Interop 根可以在 ZKChains 上设置为 bytes32(0)

ExecutorL2InteropRootStorage 合约允许操作员为尚未定义根的任何 blockOrBatchNumber 添加值为 bytes32(0) 的虚拟 interop 根。虽然这不会立即引起安全问题,但它代表了一种有风险的模式,并发出一个事件,这可能会产生误导。

考虑验证在 ZKChains 上添加的 interop 根是否不能为零。

更新:已在提交 9beb04epull request #1516 中解决。在 L1 和 L2 上添加了检查,以确保 interop 根为非零。

说明与附加信息

具有误导性的文档和注释

具有误导性的文档或注释会使开发人员感到困惑或导致对代码功能的错误假设。

在整个代码库中,发现了多个具有误导性的文档或注释的实例:

bootloader.yul 中:

  • 314 和 315 行 中,对 CURRENT_NUMBER_OF_ROOTS_IN_BLOCK_SLOT 的描述不准确地表明它返回区块中 interopRoots 的数量。但是,它提供了到目前为止处理的区块数的存储槽。同样,320 行 中对 CURRENT_NUMBER_OF_ROOTS_IN_BLOCK_BYTE 的描述也遵循相同的误导模式。
  • 337342 行中,INTEROP_BLOCKS_BEGIN_SLOTINTEROP_BLOCKS_BEGIN_BYTE 被描述为存储 interopRoots 的起点,而它们分别返回每个区块的 interopRoot 计数的起始槽和字节。

MessageHashing.sol 中:

  • getProofData 函数之上的文档字符串声明该函数对证明进行哈希处理。但是,实际功能涉及解析证明并返回最终证明数据,而不是对其进行哈希处理。

IExecutor.sol 中:

  • 可以删除 StoredBatchInfo 结构的 todo 注释,因为操作项已经实现。考虑删除它。
  • 将偏移量描述为“等于 4(用于 isService 的字节)”是不清楚的。考虑将这些替换为“等于 4(用于 ShardIdisServicetxNumberInBatch 的字节)”或类似的内容。

考虑修改文档以准确反映每个函数和变量的功能。准确的文档可以显著提高代码库的清晰度和可维护性。

更新:已在提交 678273epull request #1511 中解决。所有已识别的实例都已得到解决。

代码质量改进

在整个代码库中,发现了多个代码改进的机会:

  • getChainRoot 函数可以在返回根之前验证是否已注册 _chainId。否则,对于未注册的链,将会发生下溢错误
  • 与其定义相比,MismatchNumberOfLayer1Txs 错误的参数已反转
  • 某些错误的文档中没有正确的签名:"0x0214acb6" 应为 "0x5f1aa154","0xcea34703" 应为 "0x43e266b0",并且 "0xd7a6b5e6" 应为 "0x7774d2f9"。
  • IExecutor.sol 相比,SystemLogKeyConstants.sol 中的的定义有所不同。具体来说,缺少 MESSAGE_ROOT_ROLLING_HASH_KEY 字段。
  • MessageVerification.sol 中,在 _proveL2LogInclusion 函数中验证日志哈希与默认叶子哈希的检查应移至 _proveL2LeafInclusion 函数中,以便在证明日志的所有情况下都有效。
  • bootloader.yul 中,考虑为当前硬编码值定义批处理中已处理区块的最大数量的常量。这将有助于从语义上将其与每个批处理的 INTEROP_ROOTS 的最大数量区分开,后者目前具有相同的值

考虑解决已识别的实例,以提高代码库的清晰度和可维护性。

更新:**已在提交 5116186 的 [pull request #1517](https://github.com- 在 addChainBatchRootupdateFullTree 函数中,对共享树的更新是通过返回新根的函数完成的。此返回值被忽略,而是调用 sharedTree.root() 来获取更新后的根。考虑使用返回值来节省 gas。

  • updateFullTree 函数中,chainIndexToId[i] 的值从存储映射中读取了两次,并且似乎没有被编译器自动缓存。考虑在每个循环中手动将其缓存在局部变量中以节省 gas。

考虑实施上述建议,以提高代码库的 gas 效率。

更新:已在 pull request #1512 的 commit 081796d 中解决。

印刷错误

在整个代码库中,发现了多个印刷错误实例:

  • L2InteropRootStorage.sol第 28 行 中,NatSpec 行以 "//" 开头,而不是 "///"。考虑修复它,以便任何依赖文档解析的工具都可以成功提取它。
  • bootloader.yul第 272 行 中,“The slot starting from the L2 block [...]” 应该写成 “The slot starting from which the L2 block [...]”。

为了提高代码库的清晰度和可读性,请考虑解决上述列出的印刷错误实例。

更新:已在 pull request #1513 的 commit 741eaf7 中解决。

代码质量改进(第二阶段)

在整个代码库中,发现了多个可以改进代码的地方:

  • ValidatorTimelock 合约及其接口中,hasRoleForChainId 函数可以标记为 view
  • Executor 合约的 _calculatePrecommitmentRollingHash 函数中,assembly 可以标记为 memory-safe,因为它只访问 scratch space 和 _packedTxPrecommitments 内存数组范围内的内存。
  • bootloader 合约中,第 195 行 的注释声明只有 L2 交易存储在 CURRENT_L2_TX_HASHES 内存槽下。然而,这并不正确,因为优先级交易也存储在当前代码版本的此位置。
  • IValidatorTimelock.sol 接口中,proveBatchesSharedBridge 函数的参数可以分别重命名为 _chainAddress_processBatchFrom_processBatchTo_proofData,以便与实现保持一致。
  • ValidatorTimelock 合约中,revertBatchesSharedBridge 函数的 _l2BatchNumber 参数可以重命名为 _newLastBatch
  • Executor 合约中,一些参数类型[1] [2] [3] 已从 uint256 更改为 address。但是,“// _chainId” 注释未相应更新。在这些注释中,参数应重命名为 _chainAddress
  • ValidatorTimelock 合约中,某些注释[1] [2] 似乎已被复制粘贴,因此,鉴于上下文,这些注释是不相关的。考虑删除这些注释。
  • 对“miniblock”的引用[1] [2] 可以重命名为“l2BlockNumber”或类似名称,以提高清晰度并与代码的其余部分保持一致。

考虑实施上述代码质量改进建议,以增强代码库的清晰度和可维护性。

更新:已在 pull request #1530 的 commit 03031ec 中解决。提出的所有实例都已得到解决。

错误参数颠倒(第二阶段)

ChainTypeManager 合约中,提供给 OutdatedProtocolVersion 错误的参数顺序与错误的定义相比,其顺序似乎是颠倒的。

考虑修复此 OutdatedProtocolVersion 错误的参数顺序,以提高代码清晰度并避免混淆。

更新:已在 pull request #1535 的 commit b6b26b3 中解决。

接口和实现不匹配(第二阶段)

L1Messenger 合约的 sendToL1 函数将 bytes message 参数的数据位置定义为 calldata。但是,IL2ToL1Messenger 将参数的位置设置为 memory

考虑修复 IL2ToL1Messenger 接口,以便 sendToL1 函数参数的指定数据位置与其实现保持一致。

更新:已在 pull request #1536 的 commit c20b5f4 中解决。

排版错误(第二阶段)

在整个代码库中,发现了多个排版错误实例:

  • ServerNotifier.sol第 24 行 中,“the chain initiating migration to the ZK gateway” 应该写成 “the chain initiating migration from the ZK gateway”。
  • bootloader.yul第 2402 行 中,“The output has size of 32” 应该写成 “The output has size of 64”。
  • SystemContractHelper.sol第 455 行 中,注释中缺少单词“validate”。

为了提高代码库的清晰度和可读性,请考虑解决上述列出的排版错误实例。

更新:已在 pull request #1537 的 commit a8e76e7 中解决。

客户端报告

IExecutor 接口更改阻止链结算

在升级 ZK Chain 时,高级执行路径如下:

  • L1 合约在调用 ChainTypeManager.executeUpgrade 期间升级,该调用在 ZK Chain 的 diamond 代理上调用 diamondCut
  • 这会升级 facets(包括 Executor 合约),然后委托调用L1V29Upgrade 合约,该合约可以选择在 ZK Chain 的 diamond 代理上重新分配验证器
  • 之后,新的验证器可以例如调用 commitBatchesSharedBridge 来升级 ZK Chain 的系统合约。

然而,IExecutor 接口在代码库中被更改,而 ValidatorTimelock 实现位于 TransparentUpgradeableProxy 之后,并且与上述流程分开升级。此外,ValidatorTimelock 在所有 ZK Chain 之间共享,这些 ZK Chain 不需要同时升级。因为 ValidatorTimelock 将无法同时与新旧 IExecutor 接口兼容,这意味着它将无法再调用结算某些 ZK Chain 所需的某些函数(现在不受支持的 IExecutor 接口的一部分)。

更新:已在 pull request #1571 的 commit ed3d351 中解决。ChainTypeManager 合约现在支持两个版本的 ValidatorTimelock,它们将单独部署。ZK Chain 将在升级时更改支持的验证器,以便支持与其 IExecutor 接口兼容的 ValidatorTimelock

结论

审计下的更改主要引入了两个新功能:降低了互操作性,允许在 Gateway 上结算的 ZKChain 之间传递消息,以及快速最终性,允许第三方通过监控 L1 上的预提交来验证 ZKChain 批次的最终性。还审计了其他几个较小的代码更改,主要与修复和优化有关。

发现的主要问题与启动加载程序逻辑有关。总体而言,代码库编写良好且文档齐全。感谢 Matter Labs 团队在整个审计期间的高度响应,在需要时提供详细的解释,以及关于某些设计选择的理由的宝贵见解。

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

0 条评论

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