ZKsync SSO账户抽象审计

本文介绍了zksync SSO账户抽象的审计结果，包括安全模型、设计选择、潜在的安全问题以及高、中、低严重性的问题。文章详细列出了每个问题的描述、解决建议及其优先级，并对代码的生产就绪度进行了评估。结尾部分强调了该代码库的灵活性和健壮性，鼓励开发团队进行必要的改进与增强测试。

目录

摘要

TypeAccount AbstractionTimeline 从 2025-01-13 到 2025-02-05Languages Solidity 总问题 31 (27 已解决, 2 部分解决)关键严重性问题 0 (0 已解决)高严重性问题 1 (1 已解决)中等严重性问题 2 (1 已解决)低严重性问题 10 (8 已解决, 2 部分解决)备注和附加信息 18 (17 已解决)

范围

我们审核了 zksync-sso-clave-contracts 仓库的提交 fc0af34.

范围内的文件如下：

 src
├── auth
│   ├── Auth.sol
│   ├── BootloaderAuth.sol
│   ├── HookAuth.sol
│   └── SelfAuth.sol
├── helpers
│   ├── TimestampAsserterLocator.sol
│   ├── TokenCallbackHandler.sol
│   └── VerifierCaller.sol
├── interfaces
│   ├── IHookManager.sol
│   ├── IHook.sol
│   ├── IModule.sol
│   ├── IModuleValidator.sol
│   ├── IOwnerManager.sol
│   ├── ISsoAccount.sol
│   ├── ITimestampAsserter.sol
│   └── IValidatorManager.sol
├── libraries
│   ├── Errors.sol
│   ├── SessionLib.sol
│   ├── SignatureDecoder.sol
│   └── SsoStorage.sol
├── managers
│   ├── HookManager.sol
│   ├── OwnerManager.sol
│   └── ValidatorManager.sol
├── validators
|   ├── SessionKeyValidator.sol
|   └── WebAuthValidator.sol
├── AAFactory.sol
├── AccountProxy.sol
├── SsoAccount.sol
├── SsoBeacon.sol
├── EfficientProxy.sol
├── TransparentProxy.sol
├── batch/BatchCaller.sol
└── handlers/ERC1271Handler.sol

系统概述

待审核的代码引入了 SsoAccount 合约，这是一个更可定制的智能账户，可以集成到现有的 bootloader 账户抽象 (AA) 框架中。因此，它的调用方式与 DefaultAccount 相同，遵循以下步骤：

  1. bootloader 调用 validateTransaction 来决定是否执行该交易。
  2. 如果该交易由支付人赞助，则 bootloader 调用 prepareForPaymaster。否则，SsoAccount 合约通过 payForTransaction 自行供给交易资金。
  3. bootloader 调用 executeTransaction，这会以 SsoAccount 作为 msg.sender 发起任意调用。SsoAccount 继承了 BatchCall 功能，这为复杂的执行流程创造了机会，仅需要一次验证。

如果交易配有基于 secp256k1 曲线的 65 字节长 ECDSA 签名，则可能有效，该签名在恢复后与某个账户的 k1Owners 对应。此外，SsoAccount 合约能够将其他功能附加到自己上。这些功能可以是：

  • 在验证步骤之抢跑的Hook（validationHooks）。
  • 可以通过 任意逻辑 进行验证的自定义合约。
  • 在执行步骤前后运行的Hook（executionHooks）。

除了 SsoAccount 合约之外，待审核的代码实现了两个可以作为自定义交易验证器的合约：SessionKeyValidator 和 WebAuthValidator.

SessionKeyValidator

SessionKeyValidator 合约是一个模块，允许 SsoAccount 合约为自定义签名者创建更细粒度的执行会话。每个会话都有一个过期时间戳，并指定会话的签名者、可发送到 bootloader 合约的交易费用数额、在执行步骤期间且可以发送的转账或函数调用的数额，以及任意调用的函数选择器和参数约束。要由 SessionKeyValidator 合约验证的交易应附带将其链接到特定会话的信息，以及一个恢复到会话拥有者地址的有效 ECDSA 签名。

当前的验证器实现不需要任何验证Hook来处理该交易。此外，会话信息在存储中并不持久，意味着它必须与交易一起传递。只有会话哈希（以及使用跟踪信息）在存储中持久化。如果交易意在调用未白名单的功能，当遍历所有定义的会话政策时交易将回退。大多数功能在 SessionLib 库中实现，作为上述合约的补充。

WebAuthValidator

WebAuthValidator 合约是一个模块，允许 SsoAccount 合约通过 WebAuthn 密钥验证交易。该账户可以添加对应于多个签名域的公钥。如果附带数据可以解析成具有必要 WebAuthn 验证字段的 JSON，challenge 字段与交易的哈希匹配，并且交易签名恢复到指定签名域的公钥，则该交易被视为有效。

可升级性和 SsoAccount 部署

新的 SsoAccount 合约可以通过 AAFactory 合约 部署。

需要注意的是，每个 SsoAccount 都是可升级的，并从 SsoBeacon 合约 加载其实现，这是一种更高效的 BeaconProxy。因此，具有不同实现的账户不能独立升级，升级一个将影响所有账户。

反过来，SessionKeyValidator、WebAuthValidator 和 AAFactory 合约也都是可升级的，并将位于更高效的 TransparentProxy 实例后面，这些实例是更高效的 TransparentUpgradeableProxies。

设计选择和限制

在整个审核过程中，识别出几个设计限制。虽然这些限制不一定构成安全风险，但它们应被考虑，因为它们限制了功能或可能妨碍用户体验：

  1. 虽然 SsoAccount 合约可以授予任意签名者对细粒度执行会话的访问，但所有交易将通过同一账户流动，因此在 ZKsync 中将共享相同的 nonce。这可能导致竞争条件的出现，其中两个会话所有者提交交易，只有一个成功，而另一个回退。
  2. 如果 SsoAccount 合约未由支付人赞助，它将自称交易费用，并在执行结束时退还剩余的 gas。然而，在跟踪和强制执行交易费用支出限制时，SessionKeyValidator 合约未考虑退款，并始终 按全额交易费用减少支出限额。因此，预期会话所有者将能够在交易费用上支出比最初设定的少。
  3. SessionKeyValidator 合约要求 TimestampAsserter 合约的存在及其正确链接。由于当前实施的 TimestampAsserterLocator 合约在 ZKsync 中硬编码了 TimestampAsserter 的地址，因此在部署到其他 ZK 链时，将需要对 TimestampAsserterLocator 进行修改。
  4. 当前 SessionKeyValidator 合约的过期会话无法自动删除。除非每个 SsoAccount 合约定期清除，否则这些过期会话会积累。
  5. 重要的是要注意，验证器背后的实现是自定义的，取决于开发人员，因此不应对基础行为或集成方式作出假设。例如，在卸载过程中，如果仍然存在活动会话，SessionKeyValidator 会回退，而 WebAuthValidator 则没有此类检查。
  6. 对于 移除所有 k1Owners 或 验证器 没有任何限制。如果一个账户的所有交易验证方法都被移除，该账户将被锁定，无法验证和执行任何交易。
  7. ERC1271Handler isValidSignature 函数的实现不会运行 验证Hook。因此，由 SsoAccount 合约认为无效的签名可能通过 ERC1271 流程获通过，反之亦然。此外，SessionKeyValidator 合约 不支持此签名验证方法，因此利用此方法的所有交易在 ERC1271 流程中将无效。
  8. 在 WebAuthValidator 交易验证流程中，可能出现 提供的 clientDataJSON 具有相同 WebAuthn 密钥的多个实例。由于 JSONParserLib 库内的实现细节，使用 at 函数 会返回最近遇到的密钥的值。因此，如果提供的 clientDataJSON 对象同时具有有效和无效的 challenge 值，交易验证将依赖于哪个 challenge 值首先出现。
  9. 在 WebAuthValidator 交易验证流程 中，同一公钥可能对应多个源域。
  10. 一旦Hook或验证器被添加到 SsoAccount 合约的链表中，将调用Hook或验证器的 onInstall 函数。重要的是要注意，作为参数传递的 initData 并不强制要求不能为空（ [1], [2]）。这一细节在实现Hook或验证器时需考虑，并且在不希望空初始化数据的情况下应回退。
  11. removeHook 和 unlinkHook 函数 会将Hook从相应的集合中移除，然后调用 onUninstall 函数。任何需要调用 onUninstall 函数并检查它们是否附加到账户的Hook可能会回退，因为Hook的地址将被移除，并且不再显示为附加。
  12. 在通过 AAFactory 合约部署新账户时，initialize 函数 不需要任何验证器或 k1Owner 通过，这将导致账户被锁定。

安全模型

WebAuthValidator 合约的交易验证流程使用 Solady 的 JSONParserLib 库。需要注意的是，JSONParserLib 库可以在 JSON 格式错误或缺少请求的字段时回退交易。在多个方法中，输出验证委托给使用该库的合约。这些情况 并未在审核的代码库中明确处理，尽管没有发现攻击模式。

SsoAccount 合约可以附加其他验证或执行Hook，这将改变交易工作流的动态。由于协议不限制也不定义这些Hook应如何至少行为，因此Hook可能会实施不当。可用验证器可以提供一些参考，但没有合适的Hook合约作为榜样。

继续标准化，一些调用没有存在已定义的指南，例如 onInstall 或 onUninstall 函数。与Hook和验证管理相关的敏感功能可能会在不应该调用时被调用，这可能会导致基础Hook和验证合约在假设某些管理工作流的情况下出现问题。

特权角色和信任假设

在审核过程中，提出了以下信任假设：

  • 一个新的 SsoAccount 可以 由任何人部署，并在此时设置初始的 validators 和 k1Owners。

  • 只有 bootloader 能调用 SsoAccount 的 validateTransaction、payForTransaction、prepareForPaymaster 和 executeTransaction 函数。

  • 在正确验证交易后，SsoAccount 合约将 执行预定操作，无论是在自己身上还是在任意目标上。只有账户本人可以触发 BatchCall 功能。

  • 该账户可以添加或移除 Hook、k1Owners 和 验证器。

  • 附加到账户上的Hook可以 在交易验证、执行之前或执行之后执行操作。

  • 在 SessionKeyValidator 合约中，账户可以 创建和授予会话，以及 撤销任何以前授予的会话。由于 SessionSpec 结构 的复杂性，创建会话的人被信任提供有效会话信息，合理的转账和函数调用限制，以及约束索引设置到目标函数参数的正确偏移位置。此外，确保会话对同一 target 地址或 target + function selector 组合没有多个政策是至关重要的，因为这可能会导致跟踪支出限制时出现意外行为。此外，会话 未在链上完全验证，并且仅检查 几个项目。

  • 在 WebAuthValidator 合约中，账户可以 存储用于自定义签名域的公钥，从而通过签名恢复到正确的公钥来验证交易。

  • SessionKeyValidator、WebAuthValidator 和 AAFactory 合约的 TransparentProxy 所有者可以更改底层实现。SsoBeacon 的所有者能够更改 SsoAccount 合约代理的实现。他们被信任以符合协议的最佳利益行事，并且不进行恶意活动。每个账户的所有者被信任彻底验证它附加到其账户的每种验证机制。这包括 k1Owner 和/或自定义验证器和Hook，因为这些参与者可能促进恶意交易的正确验证，或者相反，有如此严格的交易验证逻辑，以至于系统变得锁定，并失去移除这些参与者的能力。

生产准备情况

审计过程中识别出多个可以改善的领域，以进一步增强代码库的质量和安全性：

  • 代码中留有许多 TODO 和问题注释（例如 更新费用限额），指向多个改进方向、必要的测试或现有的有限/不完美功能。
  • 尽管已经开发了一般的技术文档，但对于外部Hook或验证合约需遵循的规范的文档非常有限。此外，作为输入传递的一些数据没有适当记录，可能会错误地编码数据（例如，政策中的约束），以至于无法如预期那样解码，导致预期交易的错误执行或其回退。
  • 代码库可以受益于扩展当前的测试套件，以涵盖更多边缘案例，以及复杂流程，如有多个验证和执行Hook的账户，或通过批量操作添加或移除Hook。此外，代码库可以受益于集成测试，以验证 SsoAccount 合约在与 bootloader 和包围的 ZKsync 系统集成时是否按预期工作。
  • 由于代码库在将Hook或验证器实现和附加到 SsoAccount 合约时提供了显著的灵活性，开发人员应特别注意其设计选择和限制，并确保进行全面测试，以验证整个交易流程。

高严重性

缺乏输出验证可能会锁定 SsoAccount

在整个代码库中，调用 EnumerableSet 库的 add 和 remove 方法的功能未检查返回值，并错误地假设代码将在出现错误时回退。这使得以下情况成为可能：

  • 当 移除或解绑Hook 时，调用者可能会提供不正确的 isValidation 标志。代码将尝试从错误列表中移除执行Hook，失败默默无闻，但仍然继续调用Hook合约中的 onUninstall 函数。根据Hook的实现，后续的交易验证或执行可能会回退，因为从Hook的角度来看，它不再附加到 SsoAccount 合约或已清除所需状态。这可能导致账户的无限锁定。考虑验证 add 或 remove 调用的返回值，以防止默默失败。此外，当前实现允许一个Hook同时作为验证和执行Hook，由于这两种接口均具有 onInstall 和 onUninstall 函数，从两个列表中添加或移除Hook可能会导致意外行为。因此，考虑强制要求Hook合约仅用于验证或执行。
  • 当 添加或移除 k1Owner 地址 时，调用者可能尝试添加一个重复的 k1Owner 地址或移除一个未拥有此角色的地址。虽然 _k1Owners 集合将保持不变，但 K1AddOwner 和 K1RemoveOwner 事件将仍然被触发，这可能会影响跟踪它们的链外索引器。考虑验证 add 或 remove 调用的返回值，并在调用失败时回退。
  • 当 添加或移除模块验证器 时，调用者可能尝试添加一个重复的模块验证器或移除一个未拥有此角色的地址。类似地，尽管集合将保持不变，但将会调用 onInstall 和 onUninstall 函数，可能会因实现的不同而产生意外行为。考虑验证 add 或 remove 调用的返回值，并在调用失败时回退。

更新： 已在 pull request #260 和 pull request #311 中解决。该请求还引入了执行Hook可以返回 bytes32 context 从 preExecutionHook 调用，并将其传递给 postExecutionHook 调用的能力。

中等严重性

交易可能在非预期的时间内执行SessionKeyValidator 合约 为 SsoAccount 提供了创建自定义转账和调用策略的会话功能。这些策略可以强制限制会话所有者通过 transfer 或函数调用发送的资金数量。限制可以针对 整个会话的生命周期 或作为 每个 period 的额度 指定，允许会话所有者每 period 秒仅支出 amount 的资金。在发送交易时，用户将附加额外的 calldata 到他们的调用中，该数据解码为他们打算使用的会话的 规范和希望从中支出资金的 periodIds。根据会话的剩余额度，交易可能会失败验证。

重要的是要注意，periodIds 不包括在交易的哈希中，因为它被附加在 transaction.signature 字段的末尾，而该字段在 编码过程中不被使用。因此，可以在未经用户同意的情况下进行修改。如果验证失败的交易中，会话所有者打算仅从 periodId 为 10 的时期支出资金，则恶意行为者可以在内存池中观察到该交易，并以不同的 periodIds 重新发送该交易，从而可能实现执行并消耗来自意外期间的额度。类似的逻辑和攻击模式可以应用于 transaction.signature 中的任何信息，该信息不是 signature 本身，而是用于验证过程的辅助数据，例如 validator 字段。

考虑添加一机制以保护免受不同实体提交具有不同参数的相同交易进行验证的情况。

更新： 确认，会解决。Matter Labs 团队表示：

periodIds 并不是用来与例如 Uniswap 的交易截止日期类似地进行语义解释。它们仅作为辅助数据，能够在验证期间计算 block.timestamp / period。如果可以直接使用 block.timestamp，那么就没有必要提供 periodIds，因此也就不会有此类问题。periodIds 不被签名，因此用户/开发者不应将其视为保证该交易将在提供的周期内严格执行。如果用户/开发者关心某个失败的交易可以通过不同的 periodIds 进行重放，他们应该简单地发送另一笔交易以增加 nonce，从而防止潜在的重放。然而，关于 validator 和 validatorData 一般来说不被签名的观点确实有效，可能会在未来的模块中成为攻击矢量。目前，在 ZKsync 上的任何允许的交易类型中，没有办法允许对任意辅助数据进行签名。我们会确保在开发者文档中清晰地反映这一点，以防止未来模块中潜在的 validatorData 滥用。

添减执行Hook引起意外行为

通过 runExecutionHooks 修饰符，附加到 SsoAccount 合约的每个执行Hook都将执行逻辑 在执行前 和 在执行后。如果交易添加或移除执行Hook，当前的实现可能无法正常工作：

  • 如果移除执行Hook，则整个交易将回滚，因为 totalHooks 变量 不会更新，且 postExecutionHook 的 for 循环 将越界。在实践中，这导致无法移除执行Hook。此外，由于 OpenZeppelin EnumerableSet 库 在更新集合后没有排序保证，因此 postExecutionHooks 可能以不同的顺序执行。
  • 如果添加执行Hook，则同样的排序保证问题适用。此外，可能新添加的Hook的 postExecutionHook 函数被执行，而旧的Hook将被忽略。

考虑审查执行Hook的流程并确定在预执行和后执行之间添加或移除Hook时的预期行为。如果仅期望运行相同的Hook，请考虑缓存 totalHooks 变量和Hook列表。

更新： 在 拉取请求 #281 中已解决。

低严重性

使用 error 关键字命名变量

在 ERC1271Handler 和 SsoAccount 合约中，使用 error 关键字来命名一个变量，用于存储来自 ECDSA tryRecover 的错误返回值。

然而，error 关键字与 Error 相似，这在 Solidity 中是定义自定义错误的保留字。这样可能在维护代码时导致混淆和潜在的错误。考虑将该变量重命名为可以更好地反映其用途的其他名称。

更新： 在 拉取请求 #292 中已解决。

存储插槽指向 Clave 协议

SsoStorage 库定义了 SsoAccount 合约使用的所有不同变量的存储结构。该结构存储在 预计算的插槽 中，以防止冲突并减少攻击面。然而，使用的插槽对应于 Clave 协议。

考虑将存储插槽更改为专门针对 ZKsync SSO Account 项目的其他插槽。此外，考虑记录如何计算该哈希，就像 Clave 协议所做的。

更新： 在 拉取请求 #269 中已解决，提交 52bc7f9 和在 拉取请求 #306 中已解决。

Gas优化

在整个代码库中，发现多个Gas优化的机会：

  • 从 ERC1271Handler 合约的 isValidSignature 函数和 WebAuthValidator 合约的 validateSignature 函数的 signature 参数可以声明为 calldata 而不是 memory。考虑在适当的地方使用 calldata。
  • 在 SsoStorage 库中，__gap_0, __gap_2 和 __gap_4 变量 遵循非连续命名，并且冗余。可以将它们合并为一个单独的 __gap 变量，该变量更便宜，但仍能防止将来的升级来带来的问题。考虑改用单个 __gap 变量，或记录此类决策的原因。
  • 在 SessionKeyValidator 合约中关闭会话时，状态会被标记为 关闭，从而防止对交易的验证或会话的重新创建。同时，其余的 SessionStorage 结构 将无法通过现有实现访问。考虑删除特定账户的不必要状态，从而在关闭会话时回收Gas。
  • 在 WebAuthValidator 合约的 webAuthVerify 函数 中，rs[0] 和 rs[1] 的 bytes32 变量 的值不能小于 0。对于当前实现，考虑要求它们为零，而不是小于或等于零。

在进行这些更改时，目标应是在Gas优化和可读性之间达到最佳折衷。拥有易于理解的代码库减少未来出错的机会，并提高社区的透明度。

更新： 在 拉取请求 #294 中部分解决。Matter Labs 团队表示：

关于第三点，删除状态除了状态之外，我们必须将 StorageSpec 传递到 revokeKey 中，因为无法以其他方式知道我们想要删除的策略的目标和选择器。因此，必须将 StorageSpec 传递给 revokeKey 可能会对其他系统组件产生负面影响，这将使得用户更难撤销会话。因此我们决定不改变我们的做法。

访问控制的冗余关联

Auth 合约 从 BootloaderAuth、SelfAuth 和 HookAuth 合约继承访问控制修饰符。然后 Auth 被继承到 HookManager、ValidatorManager 和 OwnerManager 合约。但是，HookAuth 功能 没有被使用，因为 ValidatorManager 合约只使用 onlySelf 修饰符，而 SsoAccount 合约只使用 onlyBootloader 修饰符。这意味着每个从 Auth 继承的合约仅会使用其中一项特定功能，导致继承冗余代码，增加维护工作量并减少可读性。

为了提高代码库的可维护性和清晰度，考虑删除 Auth 并修改每个需要身份验证的合约，仅继承必要的身份验证修饰符，正如 BatchCaller 合约 与 SelfAuth 合约所做的那样。

更新： 在 拉取请求 #275 中已解决。

不安全的 ABI 编码

使用 abi.encodeWithSignature 或 abi.encodeWithSelector 生成低级调用的 calldata 并不是不常见的做法。然而，第一个选项不是错别字安全的，而第二个选项则不是类型安全的。结果是，这两种方法都容易出错，应被认为是不安全的。

在整个代码库中，发现了多个不安全的 ABI 编码实例：

考虑将所有不安全的 ABI 编码实例替换为 abi.encodeCall，它会检查提供的值是否确实符合被调用函数所期望的类型，并避免因打字错误而导致的错误。

更新： 在 拉取请求 #269 中已解决，提交 dd96b2e 和 69c614a 中已解决。

缺失或不完整的文档

在整个代码库中，发现了多个缺失或不完整文档的实例：

  • SessionKeyValidator 合约的 validateSignature 函数 的文档没有详细说明为什么这个特定的验证器不应该用于签名验证。
  • AAFactory 合约的 beacon 存储变量 没有文档，考虑说明它将指向 SsoBeacon 合约的一个实例。
  • 考虑增加有关 VerifierCaller 合约的 verifier 地址 的更多文档，详细说明预编译代码在哪里可以找到以进行分析。
  • 考虑围绕 WebAuthValidator 合约的验证过程 添加更多文档。例如，为什么对 r 和 s 的验证检查 重要性、authenticatorData 的第 33 个字节的含义是什么 特殊性，以及为什么 首尾两个次要标志比特是特殊的; 详细说明仅 type、challenge、origin 和 crossOrigin 字段重要的原因，以及其他字段为何与链上实现的其余部分无关。
  • 在 WebAuthValidator 合约 中，at 方法声明 "重复的键，最后带键的项目将被返回"。这意味着如果 JSON 中有多个 challenge，那么如果最后的一个成功，则该过程将通过。 然而，如果成功的项在最后一个 challenge 之前，则结果将有所不同。考虑向用户记录该行为或防止由于重复类型导致的结果不确定性。

考虑审查整个代码库以查找缺失的文档字符串以及实施每个步骤缺失的文档。此外，考虑记录程序组件的任何隐藏代码假设或预期行为。

更新: 在 拉取请求 #285 和 拉取请求 #290 中部分解决。Matter Labs 团队表示：

关于第三点：“验证者调用器是与验证者无关的，可以是预编译或合约，因此将其添加到此处没有意义”

JSON 解析行为：在 JSON RFC 中重复键的行为明确由实现决定 (https://datatracker.ietf.org/doc/html/rfc8259#section-4)。虽然以前抛出模糊 JSON 的行为可能会提高严格性，但实际上，此 JSON 是由浏览器生成的，而没有开发人员的干预，因此重复键将是客户端标准的违反。查看 https://www.w3.org/TR/webauthn-2/#dictdef-collectedclientdata 以获得客户端期望的格式。

SsoAccount 可调用 updateAccountVersion 和 updateNonceOrdering

当 SsoAccount 通过 batchCall 功能 调用 DEPLOYER_SYSTEM_CONTRACT 时，函数选择器未进行验证，这与 DefaultAccount 合约 或 _executeCall 函数 中的验证相似。这将允许账户调用 updateAccountVersion 或 updateNonceOrdering，这可能会导致任意的 nonce 集成问题。

考虑添加上述检查以防止任何意外行为。

更新： 在 拉取请求 #293 和 拉取请求 #305 中已解决。

未使用的代码

在整个代码库中，发现了多个未使用或冗余代码的实例：

为了改善代码库的清晰度和质量，考虑审查上述实例并删除任何未使用或冗余的代码。

更新： 在 拉取请求 #276 中已解决。

从批量执行的回执被忽略

当 批量调用失败的某一项 且 allowFailure 设置为 true 时，revert 不会被传播，执行将继续进行。

然而，没有关于哪些调用发生了回滚的信息被发出。此外，从失败调用返回的数据也被忽略，可能对于调试来说很重要。考虑发出一个事件，该事件指定了回滚调用的索引和返回的数据。

更新： 在 拉取请求 #293 和 拉取请求 #313 中已解决。需要注意的是，getReturnData 函数将自由内存指针更新到不对齐到 32 字节倍数的位置。虽然这目前并不会构成安全风险，但如果代码被修改以至于数据从未对齐的位置开始存储，再从对齐的位置读取，则可能会出现问题。

SsoAccount 合约部署可能遭到前置攻击

任何用户都可以通过 AAFactory 合约 部署 一个新的 SsoAccount 合约。该函数需要 _salt 和 _uniqueAccountId 作为输入，以用于 create2 调用。

然而，任何其他用户都可以在内存池中观察到该交易并进行前置攻击，执行部署并阻止原始部署。这既是拒绝服务攻击，也是资金丢失的攻击矢量：用户可以将部署与转移资金到预测的地址配对，并意外地将资金发送给一个他们不控制的 SsoAccount。

考虑通过添加一个存储变量作为 uniqueAccountId，并随着每次成功部署进行递增，以缓解这种情况。此外，考虑将 msg.sender 作为盐的一部分。

更新： 在 拉取请求 #295 和 拉取请求 #309 中已解决。

备注与补充信息

使用次优解码函数

在 SsoAccount 合约的 _validateTransaction 函数中，对 _transaction.signature 字段的解码 是通过 SignatureDecoder 库中的 decodeSignature 函数 来处理的。然而，由于在此特定情况下不需要 validatorData 输出，考虑改用 SignatureDecoder 库中的 decodeSignatureNoHookData 函数。

更新： 在 拉取请求 #269 中解决，提交 9b4591a 中解决。

命名建议

在整个代码库中，发现了多个命名改进的机会：

  • WebAuthValidator 合约的 keyExists 变量 在赋值 true 时表示密钥是新的，赋值为 false 时表示密钥是更新的。因此，考虑将变量名称更改为 keyIsNew 以避免对该输出的误解。
  • OwnerManager 合约 中的事件和函数被命名为 k1<...>Owner（ k1AddOwner, k1IsOwner, K1AddedOwner 及其他)。由于它们引用一个名为 k1Owner 的实体，考虑更改名称以符合 k1Owner<...> 和 <...>K1Owner 格式。这也将与代码库的其余部分更加一致（ HookAdded、addModuleValidator）。

更新： 在 拉取请求 #269 中解决，提交 a39df4d 和在 拉取请求 #308 中解决。

间接导入

在整个代码库中，发现多个从其他包间接导入的依赖实例：

间接导入可能会引起混淆，并可能导致底层文件版本管理的问题。为了改善代码库的可读性和减少维护代码时引入的潜在错误，考虑解决以上提到的实例，并审查代码库中的其他情况。

更新： 在 拉取请求 #267 中已解决。

排版错误

在整个代码库中，发现多个排版错误的实例：

  • 在 IHookManager 中，“it's” 应为 “its”。
  • 在 ISsoAccount 中，“are contract” 应为 “are contracts”。

考虑修正任何排版错误的实例，以提高代码库的清晰度和可读性。更新： 在 pull request #269 中已解决，提交 2c6f44c。

实现与接口不匹配

ISsoAccount.initialize 函数 的接口和实现不匹配，因为“owners”函数参数的名称不同：k1Owners 和 initialK1Owners。

考虑将这两个名称统一，以提高函数的清晰性。

更新： 在 pull request #277 中已解决。

缺少 SPDX 许可证标识符

IModule.sol 文件缺少 SPDX 许可证标识符。

为了避免与版权相关的法律问题，并遵循最佳实践，考虑按照 Solidity 文档 的建议，在文件中添加 SPDX 许可证标识符。

更新： 在 pull request #262 中已解决。此外，在 pull request #304 中添加了 pragma solidity ^0.8.24; 指令。

功能可见性过于宽松

在代码库中发现多处函数具有不必要的开放可见性：

为了更好地传达函数的预期用途，考虑将函数的可见性更改为只需满足要求的宽松程度。

更新： 在 pull request #278 中已解决。

在映射中缺少命名参数

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

在代码库中发现多处没有命名参数的映射实例：

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

更新： 在 pull request #279 中已解决。

误导性的文档

在整个代码库中发现多处误导性的文档实例：

  • IOwnerManager 接口 中的 k1AddOwner 函数的文档字符串暗示这些函数可以由白名单模块调用，但事实并非如此。
  • HookManager 和 OwnerManager 合约的文档字符串暗示地址存储在链表中，实际上它们存储在枚举集合中。
  • 以下注释提到初始化时的 hooks，而实际上只设置了 moduleValidators 和 k1Owners。

考虑解决这些误导性文档实例，以提高代码库的质量和清晰性。

更新： 在 pull request #287 中已解决。

自定义错误使用不一致

自 Solidity 版本 0.8.4 起，自定义错误提供了一种更简洁、更节省成本的方法来向用户解释操作失败的原因。尽管在某些情况下已使用自定义错误，但建议始终一致地使用它们。

AAFactory.sol、SessionKeyValidator.sol、SessionLib.sol、TimestampAsserterLocator.sol 和 WebAuthValidator.sol 文件中使用了 require 消息。考虑将它们更新为始终使用自定义错误。

更新： 在 pull request #298 中已解决，提交 a955d59。

未解决的 TODO 注释

在开发过程中，拥有描述详细的 TODO 注释将使跟踪和解决问题的过程更容易。然而，如果未及时解决，这些注释可能会老化，并且对系统安全的重要信息可能会在发布到生产时被遗忘。因此，此类注释应在项目的问题待办事项中跟踪，并在系统部署之前解决。

在代码库中发现多处 TODO 注释实例：

  • SessionLib.sol 中 247 行 的 TODO 注释
  • SsoAccount.sol 中 75 行 的 TODO 注释

此外，代码库中有些地方，例如 CallSpec 结构体，对当前实现有疑问，并考虑是否应对其进行进一步更改。理想情况下，在进入生产之前，所有 TODO 注释和对代码库的开放问题都应得到实施和解决。

考虑删除所有 TODO 注释，并将其跟踪在问题待办事项中。或者，考虑将每个内联 TODO 链接到相应的问题待办事项条目。

更新： 已解决。Matter Labs 团队表示：

这些问题已在待办事项中进行跟踪，但注释仍保留在代码库中以提供额外上下文。

修改后的合约仍将 Clave 视为作者

HookManager 合约 和 IValidationHook 和 IExecutionHook 接口相较于从 Clave 派生的原始代码已被更改，但它们仍将 Clave 视为作者。

在修改后的合约中，考虑说明虽然 Clave 是最初的作者，但自那时起已经进行了广泛修改。

更新： 在 pull request #288 中已解决。

隐式转换

在 WebAuthValidator 中，进行隐式转换以将 r 和 s 的 bytes32 值与显式的 uint256 零进行比较。

为了提高代码的可读性，考虑明确地将 bytes32 转换为 uint256，就像 OpenZeppelin 的 P256 库 所做的那样。

更新： 在 pull request #286 和 pull request #310 中已解决。

使用 rawVerify 函数可予以改进

WebAuthValidator 合约中的 rawVerify 函数 被用于严格 测试交易签名的有效性，无需经过 其他交易验证步骤。需要注意的是，与 validateTransaction 函数相比，rawVerify 函数并未接收 authenticatorData、clientDataJSON 和 rs 作为参数，而是接收 已经签名的 message。这造成了不一致性，增加了用户体验的复杂性，并可能由于两个函数所需的不同输入而引入错误。由于 _createMessage 函数 是 private，因此用户必须自己实现获取消息的方法。

为了减少在链下消息创建过程中的错误概率，考虑更新 rawVerify 函数，以接收一个 fat 签名作为参数，然后首先将其解构为 authenticatorData、clientDataJSON 和 rs，再创建消息并传递给 callVerifier。

更新： 在 pull request #312 中已解决，提交 4004ec0。用于测试目的的函数已被删除。Matter Labs 团队表示：

已删除以减少混淆。我们可能会稍后回到覆盖建议的新的测试（更大更改）中，而不是独立的预编译流程比较。

SessionKeyValidator 的 validateTransaction 函数忽略签名参数

SessionKeyValidator 合约中的 validateTransaction 函数解码 交易的 signature 字段，而不是解码 提供的 signature 参数。尽管这并不构成安全风险，但与 WebAuthValidator 的实现 不一致。

考虑在所有情况下通过解码提供的 signature 字段来实现代码行为的一致性。

更新： 在 pull request #307 中已解决。

合约内部顺序不一致

在代码库中发现多处合约函数顺序不一致的情况：

为了改善项目的整体可读性，考虑按照 Solidity 风格指南（函数顺序）建议的标准化顺序，或者按逻辑顺序排序。

更新： 已确认，将解决。Matter Labs 团队表示：

由于重新排序可能会干扰正在进行的开发（因为它会引入许多冲突），因此决定推迟此项工作。

规范与实现之间的不一致性

IModule 接口 声明，调用 onInstall 函数“在错误时必须回退（例如，如果模块已启用）”。然而，当前实现允许 WebAuthValidator 和 SessionKeyValidator 合约能多次调用该函数，当从 SsoAccount 合约直接调用时，尽管验证器已启用且输入数据不同。

为了保持代码库的一致性，考虑限制对已经启用的验证器的调用，或重新定义规范。

更新： 在 pull request #298 中已解决，提交 f13d0bd。

验证未能提前失败

在 SessionKeyValidator 合约的 会话创建 中，“expiresAt”参数用于设置 minuteBeforeExpiration 变量为0或在提供的到期前一分钟。如果 minuteBeforeExpiration 为0，则交易将在 TimestampAsserter 合约的 assertTimestampInRange 函数中回退。

考虑通过添加 if 语句，在 sessionSpec.expiresAt 小于60时显式回退该交易，以从而提前失败。

更新： 在 pull request #298 中已解决，提交 a955d59。

结论

正在审查的代码引入了一种创新且模块化的智能合约账户设计，支持使用灵活的交易验证方法和执行Hook进行自定义。这些账户可以在不受限制的情况下创建和初始化，无缝集成到 ZKsync 账户抽象流程中，就像现有的 DefaultAccount 一样。

该项目在将Hook或验证器实现和附加到 SsoAccount 合约时提供了显著的灵活性，唯一要求是存在 onInstall 和 onUninstall 方法。虽然这在支持创造性开发方面非常方便，但这种实现的自由性在与 SsoAccount 合约集成时可能会导致错误。因此，对 SsoAccount 合约有特权权利的地址应在附加新的Hook或自定义验证器之前进行研究和尽职调查。

此外，尽管现有的测试套件覆盖了广泛的场景，但仍有进一步改进的空间。扩大测试覆盖面并加强套件有助于确保遵循规范，同时识别实施中的潜在错误和边界情况。

我们鼓励 Matter Labs 团队实施本次审计中建议的修复。这些修复包括通过全面的单元、集成和向后兼容性测试来增强测试套件，并解决未完成的待办事项。然而，代码库感觉相当稳健，具有直接的流程和有用的文档。Matter Labs 团队在整个过程中表现出了卓越的响应能力和合作精神，及时解决问题并提供宝贵的见解。伴随代码库的技术文档对理解 ZKsync SSO 组件的高层次架构及其可升级性至关重要。

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

0 条评论

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

OpenZeppelin

如果觉得我的文章对您有用，请随意打赏。你的支持将鼓励我继续创作！