Matter Labs GuardianRecoveryValidator 审计

该文档是对 matter-labs/zksync-sso-clave-contracts 代码仓库的审计报告，审计重点关注了 SsoAccount 合约及其相关的 GuardianRecoveryValidator 和 WebAuthValidator 合约。

目录

概要

类型账户抽象时间线从 2025-03-05 到 2025-03-17 语言 Solidity 总问题 34 (28 个已解决，3 个部分解决) 严重问题 0 (0 个已解决) 高严重问题 0 (0 个已解决) 中等严重问题 6 (2 个已解决，3 个部分解决) 低严重问题 9 (7 个已解决) 说明 & 附加信息 19 (19 个已解决)

范围

我们审计了 matter-labs/zksync-sso-clave-contracts 仓库，提交版本为 c7714c0。

以下文件在审计范围内：

 src
├── TransparentProxy.sol
├── interfaces
│   └── IGuardianRecoveryValidator.sol
└── validators
    ├── GuardianRecoveryValidator.sol
    └── WebAuthValidator.sol

还发现了一些仓库中超出范围的文件中的问题。

系统概述

审查的代码引入了对 SsoAccount 合约的更改，这是一个更可定制的智能账户，可以插入到现有的 bootloader 账户抽象 (AA) 框架中。审查的是自上次协议审计以来所做的增量更改：

  • TransparentProxy.sol: 添加了在构造阶段传递数据的功能，这些数据将与实现的逻辑代码一起使用。
  • IGuardianRecoveryValidator.sol: 添加了用于新型 guardian 验证器模块，符合 IModuleValidator 接口定义的新接口。
  • GuardianRecoveryValidator.sol: 添加了 guardian 验证器的新实现。
  • WebAuthValidator.sol: 添加了原始 WebAuthValidator 合约的适配版本，以允许多通道密钥，这主要是由于将 credentialId 引入到存储结构中。

GuardianRecoveryValidator

该验证器负责提供一种机制来恢复对 SsoAccount 的访问，以防任何其他验证方法丢失。设置账户并将其用作安全措施的过程包括以下步骤：

  1. 通过将 WebAuthValidator 和 GuardianRecoveryValidator 合约添加为模块验证器，将 SsoAccount 合约附加到它们。
  2. 调用 GuardianRecoveryValidator 合约的 proposeValidationKey 函数，以在 SsoAccount 合约的交易执行工作流程中提议一个 guardian。
  3. guardian 必须通过调用 GuardianRecoveryValidator 合约的 addValidationKey 函数，从其账户接受提议。

然后，在需要恢复的情况下，该特定 SsoAccount 的 guardian 负责通过传递相应的账户以及哈希后的 originDomain 和 credentialId 值，和要添加到 WebAuthValidator 合约中的 rawPublicKey 来启动恢复。使用哈希版本是为了尝试使用提交-显示模式来混淆数据。此时，开始 24 小时的等待期，在此期间，正在恢复的账户可以取消恢复过程。

一旦达到恢复窗口（启动恢复后 24 到 72 小时），任何人都可以创建一个 Transaction，调用 WebAuthValidator 合约上的 addValidationKey 方法。如果提供了不同的交易数据，则执行将在 Bootloader 层还原。用于此调用的参数（credentialId、originDomain 和 rawPublicKey）必须与启动恢复时提供的参数完全匹配。如果满足这些条件，GuardianRecoveryValidator 合约将成功验证交易，从而允许代表 SsoAccount 合约继续执行。

使用恢复请求时，它会自动删除，防止再次执行。但是，过期但未使用的恢复请求不会自动删除。由于无法执行过期的恢复请求，因此必须使用新的恢复请求覆盖它，这也需要等待 24 小时的取消期。附加到此验证器的 SsoAccount 合约可以选择随意修改和删除 guardian，但只能通过可验证的 Transaction。

安全模型和信任假设

SsoAccount 合约的所有者信任 guardian 的善意，他们会选择启动恢复过程。未能做到这一点可能会严重影响恢复此类账户访问的能力，即使有诚实的 guardian 为该账户注册。

此外，恢复账户的过程依赖于另一种验证器 WebAuthValidator 合约的补充使用，该合约必须是功能性的并且附加到该账户。否则，guardian 及其验证器将无法成功完成他们的工作。

特权角色

分配给账户的 guardian 可以：

  • 当账户提议他们作为 guardian 时，接受他们的角色。
  • 启动/更新他们作为 guardian 的特定账户的恢复过程。

启动恢复后，任何人都可以将 Transaction 使用 GuardianRecoveryValidator 作为其验证器，并构造参数以包含传递的 publicKey 到 WebAuthValidator 合约。

设计选择、局限性和集成问题

  • 尽管 SsoAccount 合约模仿 Default 账户的 Bootloader 流程，但它不模仿 Default 账户提供的 EOA 行为。这意味着在调用受 onlyBootloader 修饰符保护的函数或调用未实现的功能时，可能会导致还原，而不是返回空数据。如果未正确处理这些行为，则先前使用 DefaultAccount 合约的协议在切换到新的 SsoAccount 合约时可能会遇到问题。
  • 提交恢复请求时，originDomain 和 credentialId 都使用了哈希版本来混淆它们的值。
  • 卸载过程可以使用空的 data 输入调用，该输入不会从合约中删除/取消链接任何与 guardian 相关的数据。此外，没有限制发送部分 data（这意味着，不是完整的 hashedOriginDomain 列表）。
  • 该过程期望 WebAuthValidator 合约附加到相应的 SsoAccount 合约，但没有强制执行或验证是否正在执行此操作。

中等严重性

迭代 Hook 时可能出现拒绝服务

runExecutionHooks 修饰符 和 runValidationHooks 函数 都迭代集合中的所有元素，随着集合的增长，可能会导致过度的 Gas 消耗。runExecutionHooks 修饰符包含对 EnumerableSet.values() 的调用，它一次检索所有元素，这是一个具有无限 Gas 成本的操作。同样，runValidationHooks 直接迭代所有验证 Hook，如果 Hook 的数量变得太大，会使交易验证变得越来越昂贵且可能无法使用。

在改变状态的函数中使用 EnumerableSet.values() 并在验证期间迭代大型集合是危险的，因为一次性检索或处理所有元素可能会超过单个区块的 Gas 限制。如果集合增长到超过某个大小，这些操作可能会使交易执行或验证变得不可行。

考虑明确记录所有者应限制添加到集合中的执行和验证 Hook 的数量，或者对允许的 Hook 数量施加合理的上限。

更新： 在提交版本为 dff9e12 的 pull request #372 中部分解决。Matter Labs 团队表示：

这是全局 Hook 设计中已知的问题。在 Hook 接口中添加一个小的注释是目前最好的解决方案，而无需彻底修改 Hook 的工作方式。

密钥注册期间的抢跑 (Front-Running) 场景

WebAuthValidator 合约使用两个映射进行密钥注册：publicKeys 用于将密钥与 accountAddress 关联，registeredAddress 用于将密钥与 originDomain 和 credentialId 的组合链接。此设置旨在确保系统中每个 originDomain 和 credentialId 对的唯一性。但是，该实现暴露了恶意行为者可以利用的抢跑攻击问题。

问题源于合约的检查，以防止注册重复的 originDomain 和 credentialId 组合。恶意用户可以监视内存池中旨在注册新密钥的待处理交易。检测到此类交易后，他们可以执行调用以注册具有相同 originDomain 和 credentialId 的密钥，但具有不同的、任意选择的 publicKey。这会抢在合法的注册尝试之前，占用预期的插槽。

当合法用户的交易到达 onInstall 函数 时，结果达到顶峰，其中包括一个验证步骤，以确保要添加的密钥未在其他账户下注册。如果攻击者已经声明了该插槽，则此验证将失败，导致合法交易还原并阻止用户将其验证器附加到其账户。

此外，GuardianRecoveryValidator 合约利用 commit-reveal 模式来处理 originDomain 和 credentialId，以在需要它们之前混淆它们的实际值。但是，恶意用户可以索引附加到 WebAuthValidator 合约的所有 SsoAccount 合约的过去值，哈希它们的值，并且可能能够（在某些情况下）将混淆后的值与其未哈希的实际值匹配。这将允许他们抢先开始计划攻击。

为了缓解此抢跑场景，请考虑引入额外的验证机制，以安全地将注册尝试与启动账户相关联，从而防止未经授权的密钥注册插槽抢占。此外，为了改进 commit-reveal 模式，请考虑在哈希处理和提交 WebAuthValidator 合约中的承诺时，利用账户和/或 guardian 的数据，而不是在 GuardianRecoveryValidator 合约中。

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

鉴于没有弹性链运营商拥有公共内存池或共享排序器，这充其量只是未来可能出现的问题。这里存在两个设计问题，即尝试使这些信息可访问，并将恢复问题与身份验证问题分开。将来，我们可能会考虑更新检查以支持每个凭据 ID 的账户列表，这将大大复杂化 UX，但可以防止此类抢跑。

Guardian 可以覆盖恢复流程并使其失效

GuardianRecoveryValidator 合约的 initRecovery 函数 缺少一个关键的验证步骤，允许覆盖现有的恢复数据，而无需检查正在进行的恢复过程。此疏忽允许 guardian 使用不正确的数据启动新的恢复，或刷新时间戳，从而阻止或推迟预期的恢复过程。

在账户受到多个 guardian 保护的情况下，此漏洞尤其成问题。在这种情况下，单个恶意 guardian 可以通过以下方式无限期地中断恢复过程：

  1. 使用恶意或不正确的信息覆盖现有的恢复数据。
  2. 强迫善意的 guardian 使用正确的信息覆盖此恶意数据，但只能无限期地重复该循环。

此循环不仅会阻止恢复过程，而且不会删除恶意 guardian 并重新获得对账户的控制权，因为必要的 Transaction 无法由任何一方执行。

为了解决这些漏洞，请考虑实施一种在涉及多个 guardian 的恢复过程中 guardian 之间的共识机制。只有在大多数 guardian 同意提交的参数后，这种机制才会允许恢复继续进行。此外，对于具有单个 guardian 的账户，请考虑通过合并初步检查来增强 initRecovery 函数，以确定恢复过程何时仍在活动中。这些措施将大大降低覆盖和延迟的风险，从而增强恢复过程的安全性及其有效性。

更新： 在提交版本为 f2b6bbf 的 pull request #379 中部分解决。Matter Labs 团队表示：

我们决定不在本次迭代中包含 N-out-of-M 设计；相反，我们在面向用户的文档中添加了一条建议，即使用多重签名作为 guardian 来复制此行为。

onUninstall 中未能清除 pendingRecoveryData 导致重新连接后立即进行账户恢复

onUninstall 函数 可以清除与特定账户的 guardian 相关的所有数据，但它不能清除 pendingRecoveryData 映射的请求。此遗漏允许账户重新连接 GuardianRecoveryValidator 合约并在允许的时间范围内执行待处理的恢复请求。此行为与预期逻辑相矛盾，新重新连接的账户不应能够在没有等待所需延迟的情况下执行操作，例如从 WebAuthValidator 合约调用 addValidationKey 函数，因为状态预计会被清除。

考虑在 onUninstall 函数期间显式清除 pendingRecoveryData 映射的请求，以防止基于 pendingRecoveryData 映射的旧状态进行未经授权的立即账户恢复。

更新： 在提交版本为 69f4c36 的 pull request #342 中已解决。

由于缺少验证器附件导致恢复过程不完整

SsoAccount 合约允许通过 GuardianRecoveryValidator 合约配置 guardian。此设置使 guardian 能够通过 WebAuthValidator 合约添加新的公钥，从而启动旨在重新获得对账户控制权的恢复过程。但是，在当前的实现中，缺少将 SsoAccount 合约预先附加到 WebAuthValidator 合约的要求。因此，虽然 guardian 可能已正确设置并链接到 GuardianRecoveryValidator 合约，但如果 SsoAccount 合约不承认 WebAuthValidator 是系统内的模块验证器，如 此检查 所示，则尝试通过 WebAuthValidator 合约验证后续交易将失败。

此疏忽使得恢复过程无效，因为成功添加公钥并不能保证能够通过 WebAuthValidator 合约验证未来的交易。这意味着必须提前正确设置 GuardianRecoveryValidator 和 WebAuthValidator 合约，以使恢复流程按预期运行。

此外，SsoAccount 合约将所有验证器视为独立的模块，而不提供要求它们协同操作的机制。为了解决此问题并保证恢复过程的成功完成，请考虑在将 SsoAccount 合约附加到 GuardianRecoveryValidator 合约期间实施检查。这些检查应确保 SsoAccount 合约也充分附加到 WebAuthValidator 合约，从而确保恢复流程的功能。为了获得更全面的解决方案，请考虑引入一个验证 Hook，该 Hook 允许将验证器指定为协作实体，拒绝尝试中断此协作的交易（例如，仅添加 GuardianRecoveryValidator 合约或在附加两者后删除 WebAuthValidator 合约）。此外，此 Hook 可以包含已批准的模块验证器的白名单，从而增强使用它们的任何账户的安全性。

更新： 在提交版本为 9368f6b 的 pull request #343 中部分解决。尝试连接到 GuardianRecoveryValidator 的账户现在需要链接到 webAuthValidator。虽然这解决了账户未连接到 webAuthValidator 的初始问题，但它并没有完全解决该问题。将来，用户可以在连接到 GuardianRecoveryValidator 后卸载 webAuthValidator，在这种情况下，仍然会导致账户无法恢复。Matter Labs 团队表示：

我们将在 GuardianRecoveryValidator 的 onInstall 中实施一个检查，以要求在 SSOAccount 中安装 WebAuthValidator。正如审计所建议的那样，更全面的解决方案将包括扩展 ValidatorManager 功能以允许对验证器进行分组。

由于待处理的 Guardian 接受，卸载过程可能会还原

在当前的实现中，如果账户传递了它已使用的所有 hashedOriginDomain 的整个数组（对于已接受和待处理的 guardian），则账户可能无法使用 removeModuleValidator 函数 从 SsoAccount 合约中卸载 GuardianRecoveryValidator 合约。发生这种情况是因为 onUninstall 函数 未验证 guardian 是否已接受 guardian 角色（即，isReady 标志设置为 true），然后再尝试从 guardian 的 受保护账户 集中删除该账户。因此，当 guardian 有一个待处理的请求并且尚未确认其接受时，用户的账户不包括在该 guardian 的受保护账户集中。通过传递所有 hashedOriginDomain 值（包括用于待处理 guardian 的值）尝试删除会触发 remove 函数返回 false，从而导致使用 AccountNotGuardedByAddress 自定义错误 调用还原。

因此，目前，只要任何 guardian 请求仍然待处理，用户就无法完全卸载验证器，因为与这些待处理 guardian 相关的数据无法完全清除。虽然用户可能会尝试通过省略相关的 hashedOriginDomain 条目将待处理的 guardian 从卸载过程中排除，但这种方法存在风险。具体而言，待处理的 guardian 可能会随后通过 addValidationKey 函数 接受他们的角色，而账户所有者不知情。由于用户已禁用验证器模块并且可能不会主动监视此类事件，因此新接受的 guardian 可能会在未被检测到的情况下启动账户恢复过程。

如果用户稍后重新安装验证器模块，他们可能会意外地发现新的 guardian 已经到位，可能正在向 WebAuthValidator 合约添加恢复密钥。由于用户无法拒绝向其账户添加新的公钥，因此这种情况可能会升级为 SsoAccount 合约接管，从而能够执行未经授权的交易。

考虑验证 guardian 是否已通过检查 accountGuardianData 结构中的 isReady 标志来确认他们的角色，然后再从 guardian 的 guardedAccounts 映射中删除该账户，这与 removeValidationKey 函数中实施的验证类似。

更新： 在提交版本为 c36e31a 的 pull request #345 中已解决。

低严重性

可能初始化无法使用的 SsoAccount

在 SsoAccount 合约的 initialize 函数 中，未执行验证以确保数组参数为非空。此遗漏允许合约以无法执行任何操作的状态初始化，从而导致账户无法使用。

考虑添加验证检查以确保 initialValidators 或 initialK1Owners 数组中至少一个的长度为非零，然后再进行初始化。这将防止创建非功能性账户。

更新： 在提交版本为 8454fce 的 pull request #371 中已解决。

_executeCall 中数据切片期间可能发生 Panic

在 SsoAccount 合约的 _executeCall 函数 中，在处理 DEPLOYER_SYSTEM_CONTRACT 调用时，会对 _data 参数执行切片操作。此切片操作的目的是检索函数选择器，这需要至少 4 个字节的数据。但是，当前的实现未验证输入数据是否具有足够的长度。如果提供的数据短于 4 个字节，则切片操作将导致 panic，从而突然终止执行。

考虑在切片之前验证 _data 参数的长度，如果长度不足，则显式地使用有意义的错误消息还原。

更新： 在提交版本为 f476fb1 的 pull request #369 中已解决。

pendingRecoveryData 的内置 Getter 不返回 rawPublicKey

pendingRecoveryData 变量 使用 public 可见性修饰符声明，导致 Solidity 编译器自动生成一个 public Getter 函数。但是，此 Getter 仅返回结构的简单成员（hashedCredentialId 和 timestamp），并且会省略复杂的数据结构，例如 rawPublicKey。发生这种情况是因为自动生成的 public 状态变量的 Getter 不返回结构内的数组或映射成员，即使是嵌套的也是如此。有关更多信息，请参见 Solidity 文档。

由于合约已经实现了 getPendingRecoveryData Getter，因此请考虑通过降低变量的可见性来删除自动 Getter（不检索所有数据）。

更新： 在提交版本为 832372a 的 pull request #346 中已解决。

addValidationKey 中误导性的错误消息

WebAuthValidator 合约 的 addValidationKey 函数旨在添加新的验证密钥。如果该函数返回 false，则 onInstall 函数将发出 WEBAUTHN_KEY_EXISTS 错误，指示该密钥已存在。但是，由于该函数可能返回 false 的其他条件（例如，接收到 rawPublicKey 的空输入），因此此错误消息可能会产生误导。这种错误报告中缺乏特异性可能会导致混淆，因为用户可能会错误地推断出密钥存在，而实际上，该函数由于不同的验证问题而失败。

但是，由于该函数可能返回 false 的其他条件（例如，接收到 rawPublicKey 的空输入），因此此错误消息可能会产生误导。这种错误报告中缺乏特异性可能会导致混淆，因为用户可能会错误地推断出密钥存在，而实际上，该函数由于不同的验证问题而失败。

为了提高清晰度并改进错误处理，请考虑根据失败条件区分错误消息。这可能涉及引入新的错误代码，用于输入验证失败的情况，并严格保留 WEBAUTHN_KEY_EXISTS 错误，用于尝试添加重复密钥的情况。通过这样做，用户和开发人员可以更准确地诊断问题并理解合约的行为，从而带来更健壮和用户友好的体验。

更新： 在提交版本为 5f89af4 的 pull request #333 中已解决。

偏离规范

在整个代码库中，发现了多个偏离规范的实例：

缺少函数参数的检查

当执行带有 address 参数的操作时，至关重要的是要确保地址未设置为零。将地址设置为零是有问题的，因为它具有特殊的销毁/放弃语义。因此，此操作应由单独的函数处理，以防止在值或所有权转移期间意外丢失访问权限。

在 GuardianRecoveryValidator 中，发现了多个缺少零地址检查的实例：

同样，在 WebAuthValidator 的 addValidationKey 函数中，没有对 credentialId 和 originDomain 进行验证，从而允许它们为空或具有任意长度。此外，AAFactory 的构造函数目前缺少对其输入参数的验证检查。这种缺乏验证可能会在操作期间导致意外的合约回滚。例如，如果 _beaconProxyBytecodeHash 设置为空值，则 ContractDeployer 将始终回滚，导致工厂无法使用并需要重新部署。

考虑在将参数分配给状态变量之前，为参数添加适当的验证检查。

更新： 已在 pull request #349 的 b65d4cf 提交和 pull request #333 的 c9c8e79 提交中解决。

浮动 Pragma

Pragma 指令应固定以清楚地标识合约将使用哪个 Solidity 版本进行编译。

在整个代码库中，发现了多个浮动 pragma 指令的实例：

考虑使用固定的 pragma 指令。

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

我们使用浮动 pragmas 以便可以轻松地将这些基本合约与未来的编译器版本以及依赖于这些合约的其他未来软件包一起使用。这些未来版本可以提供 gas 优化或安全性增强。仅更新这 4 个文件将与项目的其余部分和其他已发布的 zksync 系统合约不一致。

缺乏公钥验证

在 addValidationKey 函数中，当前仅检查公钥以确保它非零。但是，这不能保证该密钥是椭圆曲线上的有效点，或者它以后可以按预期使用。缺乏显式验证使得可以添加无效的公钥，这可能导致无法生成有效签名的情形。这可能会影响身份验证机制并可能损害系统完整性。

考虑实施验证步骤，以确保提供的公钥是椭圆曲线上的有效点，然后再添加它。

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

也没有验证所提供的 k1 所有者密钥是否为有效密钥，或者会话 k1 密钥是否有效。虽然我们认为这是一个可能的改进，但添加密钥时真正验证它所需的额外 gas 将是非平凡的，并且会显着更改向此模块添加密钥的界面。

想法是使用已经在客户端中验证的 webauth.create 流程，并以与我们对 webauthn.get 流程使用 webAuthVerify 相同的方式进行复制以进行验证。

后续问题在此处创建：https://github.com/matter-labs/zksync-sso-clave-contracts/issues/340。

缺少或不完整的文档

文档字符串为智能合约中的合约、函数、事件和状态变量提供了必要的上下文。它们清楚地解释了预期行为、用法和相关参数，从而极大地提高了可读性、可维护性和安全审计的便利性。

在 GuardianRecoveryValidator.sol、IGuardianRecoveryValidator.sol 和 WebAuthValidator.sol 中发现了多个不完整或缺少文档字符串的实例。例如，在 GuardianRecoveryValidator.sol 文件中，诸如 onInstall 和 onUninstall 之类的函数缺少对其参数的适当文档记录。这使得开发人员或审计员难以清楚地理解这些组件的功能和含义。

考虑根据 Ethereum Natural Specification Format (NatSpec) 彻底记录所有合约、函数、事件和相关的状态变量。这应包括对与公共 API 相关联的所有参数和返回值的清晰、简洁的解释，以确保清晰度和易于审查。

更新： 已在 pull request #380 的 fd8fdc9 提交中解决。

注释 & 附加信息

SSO_STORAGE_SLOT 的非标准化存储位置

SsoAccount 合约当前使用 keccak256('zksync-sso.contracts.SsoStorage') - 1 公式设置 SSO_STORAGE_SLOT 的存储偏移量。尽管功能正常，但此方法偏离了 EIP-7201 提出的标准化方法，该方法专门用于最大程度地降低与默认 Solidity 存储插槽发生存储冲突的风险。采用此标准还可以促进未来协议升级中的优化机会，例如那些涉及 Verkle 状态树迁移的升级（如果适用于 ZKsync）。

考虑使 SSO_STORAGE_SLOT 的存储偏移量计算与 EIP-7201 中概述的标准化方法对齐，以确保更安全的存储管理。

更新： 已在 pull request #369 的 45360b4 提交中解决。

未使用的代码

在 GuardianRecoveryValidator 合约中，声明了几个自定义错误，但从未使用过：

  • PasskeyNotMatched
  • CooldownPeriodNotPassed
  • ExpiredRequest

此外，在 SsoAccount 合约中，定义了 signature 变量，但从未使用过。此外，GuardianRecoveryValidator.sol 文件 包含对 BatchCaller 和 Call 的导入，但未使用。

考虑删除上述未使用的错误、signature 变量和不必要的导入，以提高代码清晰度、可维护性和可读性。

更新： 已在 pull request #350 的 84162ef 和 66c2731 提交以及 pull request #377 的 37fe8d7 提交中解决。

冗余的存储访问

SsoAccount 合约的 validateTransaction 函数 调用 _transaction.totalRequiredBalance() 两次：一次是检查该帐户是否有足够的余额，另一次是在余额不足时引发错误。这种冗余调用增加了不必要的计算开销。

由于 _transaction.totalRequiredBalance() 在此范围内返回相同的结果，因此重复调用它是低效的。相反，将该值存储在局部变量中并重用它将提高性能和可读性。WebAuthValidator 合约中也存在类似的效率低下问题，此处 存在一个未使用的代码片段。

考虑在局部变量中缓存 _transaction.totalRequiredBalance() 的结果，并在 WebAuthValidator 合约中类似地缓存 registeredAddress。

更新： 已在 pull request #369 的 b12ebf9 提交中解决。

WebAuthValidator 中 publicKeys 的冗余 Getter

在 Solidity 中，当一个状态变量被声明为 public 时，编译器会自动为其生成一个 getter 函数。此 getter 提供对变量数据的访问权限，但可能存在一些限制，具体取决于变量类型。对于数组或映射，Solidity 生成的 getter 每次只能返回一个元素，这可能会导致合约中存在冗余的自定义 getter 函数。

publicKeys 映射变量 使用 public 可见性声明，从而提示编译器为 bytes32[2] publicKey 数组生成一个默认 getter。但是，此自动生成的 getter 每次仅返回一个元素，这不足以检索完整的密钥。因此，该合约还定义了一个单独的 getAccountKey 函数，该函数返回数组的两个元素。 这导致了冗余的功能，因为这些访问方法之一是不必要的。

考虑将 publicKeys 映射的可见性更改为更严格的可见性，例如 internal 或 private，以防止编译器生成自动 getter。这将消除冗余，并确保仅使用显式定义的 getAccountKey 函数来检索密钥。

更新： 已在 pull request #333 的 78b8bb0 提交中解决。Matter Labs 团队表示：

我们添加了注释并重新排列了布局。

接口和实现之间的不一致

IGuardianRecoveryValidator 接口及其实现存在参数命名、函数排序和缺少函数方面的不一致之处，这可能会妨碍可用性和开发人员体验。特别是：

解决这些问题将简化 IGuardianRecoveryValidator 接口，从而提高清晰度并改善开发体验。考虑修复上述实例。

更新： 已在 pull request #361 的 1be0586 提交中解决。

IGuardianRecoveryValidator 未针对开发进行优化

在 IGuardianRecoveryValidator 接口中，可以优化当前的实现，以获得更好的开发人员体验和协议互操作性。具体来说，可以在接口级别移动实现中定义的事件、错误和结构体。这种调整不仅可以简化合约的结构，还可以提高其在各种开发场景中的可用性。

将这些定义移动到接口将有助于其他合约或开发人员更有效地交互、解码或处理这些特定事件和错误。它还将简化那些希望在他们的实施中将结构体作为输入或输出传递的过程，例如对于 guardiansFor 函数。鉴于 IGuardianRecoveryValidator 是与这些结构交互的主要接口，因此将它们直接集成到接口中可以显着提高清晰度并降低与协议交互的开发人员的复杂性。

考虑重组接口以直接包含这些定义。这种方法不仅遵循合约设计中的最佳实践，而且还通过为与合约功能交互提供更直观和可访问的界面，从而显着提高了开发人员的体验。

更新： 已在 pull request #361 的 1be0586 提交中解决。

对地址转换的重构机会

GuardianRecoveryValidator 合约采用直接类型转换方法来将 uint256 转换为地址。

由于 safeCastToAddress 方法已存在于 Utils 库中，考虑在 validateTransaction 函数中使用 safeCastToAddress 方法，以减少维护工作量。

更新： 已在 pull request #351 的 934d446 和 e735710 提交中解决。

与时间相关的变量的类型不一致

在 GuardianRecoveryValidator 合约中，用于时间测量的变量类型存在不一致之处，尤其是 Guardian struct 中的 addedAt 变量 和 RecoveryRequest struct 中的 timestamp 变量。 对于类似的时间相关数据点，这种变量类型的变化可能会导致合约操作内部或外部的混淆和冲突。

为了提高代码的可读性并保持一致性，请考虑为相似的单位保持相同的变量类型。 或者，如果这是由 gas 优化导致的，请考虑记录各个 struct 上的设计选择。

更新： 已在 pull request #357 的 70b0601 提交中解决。

GuardianRecoveryValidator 中的误导性函数命名

GuardianRecoveryValidator 合约使用诸如 proposeValidationKey、removeValidationKey 和 addValidationKey 之类的函数名称，这些名称暗示了与其他验证器中的函数相似，例如在 WebAuthValidator 中的函数。 但是，GuardianRecoveryValidator 中函数的功能却大不相同，因为它们旨在管理帐户的加粗 guardian，而不是验证密钥。

这种差异可能会导致混淆和错误，因为命名约定不能准确反映函数加粗的用途。 此外，接口定义 IModule 和 IModuleValidator 没有强制执行此类命名约定，从而增加了误用或误解的风险。 另外，这些函数的参数与其在其他验证器中的同名参数不一致，从而导致不同的函数选择器。 这进一步加剧了开发人员和审计员之间产生混淆和错误的可能性。

为了减轻这些问题并提高清晰度，建议重命名这些函数，以更准确地描述它们在管理加粗 guardian 中的特定角色，而不是验证密钥。 采用清晰且描述性的命名约定将提高代码的可读性，降低出错的风险，并有助于更好地理解合约的功能。

更新： 已在 pull request #353 的 ca53654 提交中解决。

使用自定义错误

自从 Solidity 0.8.4 版本以来，自定义错误提供了一种更简洁且更经济高效的方式来向用户解释操作失败的原因。

在 GuardianRecoveryValidator.sol 中发现了 revert 和/或 require 消息的多个实例：

为了简洁和节省 gas，请考虑使用自定义错误替换 require 和 revert 消息。

更新： 已在 pull request #354 的 c09d2ff 提交中解决。

映射中缺少命名参数

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

在 GuardianRecoveryValidator.sol 中，发现了缺少命名参数的映射的多个实例：

如果上一个命名参数省略是一种设计选择，请考虑在整个代码库中保持一致的编码风格，因为在 WebAuthValidator 合约中，publicKeys 映射中的 publicKey 参数 确实有名称。 否则，请考虑向映射添加命名参数，以提高代码库的可读性和可维护性。

_更新： 已在 [pull request #358](https://github.com/matter- 在 GuardianRecoveryValidator 合约的 GuardianRecoveryValidator.sol 中，onlyGuardianOf 修饰器 在函数和函数未按可见性排序之间实现。

为了提高项目的整体可读性，请考虑根据 Solidity 风格指南 （函数顺序）标准化整个代码库的排序。

加粗更新：已在 pull request #362 的 commit ae86f43 中解决，pull request #333 的 commit 640d7c9 中解决。

函数可见性过度宽松

在整个代码库中，发现了多个具有不必要地宽松可见性的函数实例：

  • GuardianRecoveryValidator.sol 中的 initialize 函数具有 public 可见性，可以限制为 external。

  • GuardianRecoveryValidator.sol 中的 finishRecovery 函数具有 internal 可见性，可以限制为 private。

  • GuardianRecoveryValidator.sol 中的 _discardRecovery 函数具有 internal 可见性，可以限制为 private。

  • GuardianRecoveryValidator.sol 中的 guardiansFor 函数具有 public 可见性，可以限制为 external。

  • GuardianRecoveryValidator.sol 中的 guardianOf 函数具有 public 可见性，可以限制为 external。

  • GuardianRecoveryValidator.sol 中的 getPendingRecoveryData 函数具有 public 可见性，可以限制为 external。

为了更好地表达函数的预期用途，并可能实现一些额外的 Gas 节省，请考虑将函数的可见性更改为仅在需要时才宽松。

加粗更新：已在 pull request #359 的 commit 924d857 中解决。

使用 calldata 代替 memory

在处理 external 函数的参数时，直接从 calldata 读取参数比将它们存储到 memory 中更节省 Gas。calldata 是内存的一个只读区域，其中包含传入的 external 函数调用的参数。与 memory 相比，这使得使用 calldata 作为此类参数的数据位置更便宜、更有效。因此，在这种情况下使用 calldata 通常会节省 Gas 并提高智能合约的性能。

在 GuardianRecoveryValidator.sol 中，发现了多个函数参数应使用 calldata 而不是 memory 的实例：

考虑使用 calldata 作为 external 函数参数的数据位置，以优化 Gas 使用。

加粗更新：已在 pull request #360 的 commit 79d3725 中解决。

次优的 ERC-165 接口检查实现

ValidatorManager 合约的 _supportsModuleValidator 函数 目前分别检查给定的 validator 帐户是否实现了 IModuleValidator 和 IModule 接口。每个接口检查都通过 supportsERC165InterfaceUnchecked 方法使用三个 external 调用，总共导致六个 external 调用。但是，ERC165Checker 库提供了一个更有效的替代方案，即 supportsAllInterfaces 函数，该函数能够仅通过四个 external 调用同时验证两个接口。在每次初始化 SsoAccount 合约期间，利用此方法将显着降低 Gas 消耗。这同样适用于 HookManager 合约中的 _supportsHook 函数。

考虑将单独的接口检查替换为对 supportsAllInterfaces 的单个调用，提供 IModuleValidator 和 IModule 选择器，以优化 SsoAccount 合约 initialize 函数中的 Gas 使用。

加粗更新：已在 pull request #369 的 commit de51ab6 中解决。

印刷错误

在 SsoAccount 合约中 initialize 函数 的 NatSpec 注释中，abi.encode(validatorAddr,validationKey)) 代码段包含一个额外的右括号。

考虑更正上述印刷错误。

加粗更新：已在 pull request #370 的 commit aa313ab 中解决。

webAuthVerify 中冗余的哈希操作

在 WebAuthValidator 合约的 webAuthVerify 函数 中，执行 clientDataJSON 中某些字符串字段与预定义的常量字符串值之间的比较。目前，该函数使用 Strings.equal 函数，该函数每次比较时都会对提供的字符串和常量字符串进行哈希处理。由于这些常量字符串没有改变，因此这种实现不必要地重复了哈希操作，增加了执行成本。

可以通过为常量字符串值（如 "webauthn.get" 和 "false"）预先计算哈希值，并将这些哈希值直接存储在合约中来优化该功能。因此，比较只需要对输入数据进行一次哈希处理，从而避免了冗余的哈希操作。

考虑通过存储常量值的预计算哈希值来实现此方法，从而最大限度地减少哈希操作并降低 webAuthVerify 函数的总体执行成本。

加粗更新：已在 pull request #333 的 commit ce6f196 中解决。

建议

GuardianRecoveryValidator 中的代码风格

可以改进 GuardianRecoveryValidator 合约中的多个区域，以提高一致性和可维护性：

考虑实施上述建议以提高代码库的清晰度和可维护性。

结论

在审计期间，审查了新的 guardian recovery validator 合约和相关的更改。这种新的 validator 合约允许用户为其 SsoAccount 帐户指定 guardian，从而允许在用户失去对其控制权时进行恢复过程。恢复功能取决于与 WebAuthValidator 和 GuardianRecoveryValidator 合约的集成，为 ZKsync 生态系统引入了一种新的社交恢复方式。

此次审计发现了多个中等严重程度的发现，以及几个低等和提示严重程度的问题。GuardianRecoveryValidator 合约由于处于初始迭代阶段，因此还有改进的空间，例如具有更清晰的代码结构、通过组件重用减少重复代码以及优化以增强可读性并降低执行成本。此外，提供更多的描述性文档，特别是对于 WebAuthValidator 合约中的新功能，将增强清晰度和可维护性。最后，更全面的测试套件也可以提高代码库的质量，特别是通过添加更多的负面案例场景，这些场景可能会发现报告中提出的一些问题。

在整个审计过程中，Matter Labs 团队反应迅速且合作。我们感谢他们的专业精神和合作。

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

0 条评论

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

OpenZeppelin

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