WisdomTree Digital 白名单上下文审计

本文对WisdomTree Digital团队的白名单合规Oracle系统的全面审计,旨在识别潜在的漏洞和验证其遵循最佳实践的情况。审计过程中发现了20个问题,提出了许多改善建议,特别是在访问控制的粒度、代码可读性及审核合约的安全性方面。七个低严重性问题的提出使得系统在合规性和功能性上得到了增强。

目录

摘要

TypeDeFiTimelineFrom 2025-01-27To 2025-02-06LanguagesSolidity总问题20 (15 解决)关键严重性问题0 (0 解决)高严重性问题0 (0 解决)中等严重性问题0 (0 解决)低严重性问题3 (2 解决)备注与附加信息17 (13 解决)

范围

我们对wisdomtreeam/whitelist-contexts 仓库的提交4476078进行了审计。

以下文件已完全审计:

 src
├── common
│   ├── access-control
│   │   ├── AccessControl.sol
│   │   └── IAccessControl.sol
│   ├── interfaces
│   │   ├── IBeacon.sol
│   │   └── IERC165.sol
│   └── libraries
│       ├── BytesHelper.sol
│       └── StorageSlot.sol
├── oracles
│   ├── interfaces
│   │   ├── ICompliance.sol
│   │   ├── IOracle.sol
│   │   ├── IOracleBeaconUpgrade.sol
│   │   ├── IOracleInit.sol
│   │   ├── IWhitelistComplianceOracle.sol
│   │   └── IWhitelistOracle.sol
│   └── whitelist-compliance
│       └── WhitelistComplianceOracle.sol
├── proxies
│   ├── Beacon.sol
│   └── Proxy.sol
└── tokens
    ├── ERC721
    │   ├── ERC721BasicToken.sol
    │   └── ERC721SoulboundToken.sol
    └── interfaces
        ├── erc1155
        │   └── IERC1155.sol
        ├── erc721
        │   ├── IERC721.sol
        │   ├── IERC721BeaconUpgrade.sol
        │   ├── IERC721Burnable.sol
        │   ├── IERC721Enumerable.sol
        │   ├── IERC721Errors.sol
        │   ├── IERC721Events.sol
        │   ├── IERC721Metadata.sol
        │   ├── IERC721Mintable.sol
        │   ├── IERC721Receiver.sol
        │   ├── IERC721Token.sol
        │   └── IERC721TokenInit.sol
        └── erc721Soulbound
            ├── IERC721Soulbound.sol
            ├── IERC721SoulboundBeaconUpgrade.sol
            ├── IERC721SoulboundBurnable.sol
            ├── IERC721SoulboundEnumerable.sol
            ├── IERC721SoulboundErrors.sol
            ├── IERC721SoulboundEvents.sol
            ├── IERC721SoulboundMetadata.sol
            ├── IERC721SoulboundMintable.sol
            ├── IERC721SoulboundReceiver.sol
            ├── IERC721SoulboundToken.sol
            └── IERC721SoulboundTokenInit.sol

以下文件与 OpenZeppelin 库发布的 v5.0.0 进行了差异审计:

 src
└── common
    └── libraries
        └── Arrays.sol

以下文件与 OpenZeppelin 库发布的 v4.8.0 进行了差异审计:

 src
└── common
    └── libraries
        ├── Math.sol
        └── Strings.sol

执行摘要

OpenZeppelin 受 WisdomTree Digital 团队委托,对其预言机白名单系统的实施进行了全面审计,该系统允许代币标准通过白名单成员验证进行身份验证。我们的目标是识别潜在的漏洞,验证遵循最佳实践,并提供增强合约整体安全性和功能性的建议。审计仅审查了计划在链上部署的智能合约组件,并未审查任何部署脚本、配置、模拟合约或测试。

该系统包括一个白名单预言机,旨在通过验证接收方是否持有指定的 ERC-721 或 ERC-1155 合约的任何代币或直接由自定义的 WhitelistOracle 合约允许的方式来授权代币标准的转移。此外,该系统实现了两个版本的 ERC-721 代币。第一个用于白名单合规性预言机,是一种不可转移的“灵魂绑定”版本,可以铸造为单一地址且永远不能转移,旨在由白名单帐户持有。第二个是标准实施,尽管不是系统的一部分,但为更复杂的代币设计提供了基础。

该审计由两名全职审计员进行,逐行手动审计整个代码库,并使用自动化代码分析进行漏洞检测。特别是,审计验证了:

  • 所有功能按预期执行。
  • 代币合约符合 ERC-721 标准。
  • 白名单实施是稳健的,不能在正确集成到代币合约中时被绕过。
  • 外部参与者只能按预期与系统交互。
  • 遵循最佳实践的代码风格和文档。

该审计识别出 20 个问题。特别地,建议使访问控制更加细粒度,以确保每个持有特权角色的帐户只能执行一小部分操作,从而降低被破坏帐户的整体风险。提出的其余问题旨在改善代码库的可读性和可维护性,解决代码中的一些小的不一致性和冗余,并降低Gas成本。总体而言,代码库被发现结构良好,并遵循最佳实践。

系统概述

白名单预言机系统提供了一种合规框架,使代币标准能够通过白名单成员验证进行转移身份验证。该系统的核心是 WhitelistComplianceOracle 合约,它是系统验证流程的核心。此外,该项目包括两个代币实现:经典的 ERC-721 代币和不可转移的“灵魂绑定” ERC-721 代币。ERC-721 标准实现改编自 OpenZeppelin 的库,并已根据项目要求进行了修改。

白名单合规性预言机

WhitelistComplianceOracle 合约作为白名单预言机系统的中央合规验证中心。它实现了 ICompliance 接口,便利与白名单合约注册表的无缝交互。该注册表对系统至关重要,因为它维护了全面的合同列表,用于授权代币转账。这些合同可以是代币合同,例如 ERC721SoulboundTokenERC1155,如果接收方至少持有指定的代币,则允许转移,或外部的 WhitelistOracle 合约,可查询以检查转移授权。

ERC-721 灵魂绑定代币

ERC721SoulboundToken 是一种不可转移的 NFT 实现。其目的是在系统内定义成员身份和身份。通过扩展 ERC-721 标准,这种代币变体引入了“灵魂绑定”特性,确保代币不可撤回地与其初始接收地址相关联,从而防止任何转移形式,并增强系统的安全性和合规能力。这样的代币是在用户通过 KYC 或 AML 流程后铸造的。

ERC-721 基本代币

虽然在当前白名单预言机系统的范围内未积极使用,ERC721BasicToken 合约作为基础支柱,提供基本的 ERC-721 功能,补充角色基于访问控制和可升级性特性。该标准实现为更复杂的代币实现奠定了基础,也提供了基于角色的访问控制和可升级模式。

访问控制

所有组件继承了 AccessControl 合约,该合约为系统内每个合同定义特定角色,确保细粒度控制和安全性。此外,为原子升级采用 Beacon Proxy 模式,并使用 Storage Slots 模式以防止升级冲突,确保了系统的持久性和适应性。

安全模型和信任假设

白名单预言机系统通过 AccessControl 合约实现了多个特权角色。持有这些角色的人被期望是非恶意的,并以项目的最佳利益行事。

特权角色

系统引入了几个不同的特权角色,每个角色都有特定的责任和能力:

  • DEFAULT_ADMIN_ROLE

    • 在初始合约设置期间分配。
    • 有权向其他参与者分配角色,并启动对信标的升级。
    • 该角色专门保留给最多三个地址。
  • DELEGATED_ADMIN_ROLE

    • 此角色由 DEFAULT_ADMIN_ROLEDELEGATED_ADMIN_ROLE 持有者共同管理。
    • ISSUER_ROLEREGISTRAR_ROLE 角色的管理员角色。
    • DEFAULT_ADMIN_ROLE 角色被撤销时,其所有被委托角色会同时被撤销。
  • REGISTRAR_ROLE

    • 此角色由 DEFAULT_ADMIN_ROLEDELEGATED_ADMIN_ROLE 持有者共同管理。
    • 有权监督与代币相关的活动,包括铸造或销毁代币。
  • ISSUER_ROLE

    • 此角色由 DEFAULT_ADMIN_ROLEDELEGATED_ADMIN_ROLE 持有者共同管理。
    • 为尚未引入的功能而分配。

该系统还实现了一种机制,以防止对 DEFAULT_ADMIN_ROLE 失去访问权,明确要求至少一个地址始终持有该角色。

低严重性

白名单合规性预言机缺少细粒度权限控制

WhitelistComplianceOracle 合约继承了 AccessControl 合约,但仅依赖拥有 DEFAULT_ADMIN_ROLE 的拥有者来管理合约。该角色有能力升级信标地址、添加和移除地址上下文、启用和禁用预言机,以及设置合约上下文的最大数量。

考虑实施更细粒度的访问控制。

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

_查找已确认。对 WhitelistComplianceOracle 的限制仅由 DEFAULT_ADMIN_ROLE 进行管理是故意设计的,这是一种仅由我们协议内的管理员可修改的管理员级预言机。_

缺少事件发射

[setBaseURI](https://bitbucket.org/wisdomtreeam/whitelist-contexts/src/44760785af1696704cdfcc56c2ba4a17ff603fb5/src/tokens/ERC721/ERC721SoulboundToken.sol#lines-219:221) 和 [setTokenURI](https://bitbucket.org/wisdomtreeam/whitelist-contexts/src/44760785af1696704cdfcc56c2ba4a17ff603fb5/src/tokens/ERC721/ERC721SoulboundToken.sol#lines-226:229) 函数的ERC721SoulboundToken` 合约没有发射事件。这可能会使链外应用跟踪合约中的更改变得困难。

考虑在更新基础 URI 或代币 URI 时发射事件。

更新:pull request #5中的提交9d6b857中解决。团队表示:

通过为 ERC721SoulboundToken 合约的 setBaseURIsetTokenURI 函数添加事件发射,实施了审计员推荐的修复。

浮动Pragma

Pragma 指令应固定,以清晰地标识合约编译时所用的 Solidity 版本。在代码库中识别出多处使用浮动 pragma 指令 ( ^0.8.19) 的实例。

考虑使用固定的 pragma 指令。

更新:pull request #6中的提交7fefad4中解决。团队表示:

通过将代码库中的 pragma 指令从浮动更改为固定,实施了审计员推荐的修复。

备注与附加信息

授予被委托管理员角色时访问控制不一致

grantDelegateAdminRole 函数的 AccessControl 合约可以由 DEFAULT_ADMIN_ROLEDELEGATED_ADMIN_ROLE 执行。然而,相同功能的批量版本,batchGrantDelegateAdminRole 函数只能由 DEFAULT_ADMIN_ROLE 执行。

考虑调整 batchGrantDelegateAdminRole 函数的访问控制。

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

_该发现已确认。目前,batchGrantDelegateAdminRole 对于 DEFAULT_ADMIN_ROLE 的限制将保持不变,因为我们希望使我们协议中的所有访问控制实现保持一致。未来,一旦我们的协议处于访问控制升级范围内,我们可以探索修改此限制。_

冗余代码

在代码库中识别出多处冗余或不必要的代码:

考虑实施以上代码更改以改善代码清晰度并消除冗余。

更新:pull request #7中的提交49814d1 和提交9fecb7b中解决。团队表示:

通过减少 WhitelistComplianceOracleERC721BasicTokenERC721SoulboundToken 合约的 upgradeBeaconToAndCallsupportInterface 函数中的冗余,实施了审计员推荐的修复。

使用 int/uint 而不是 int256/uint256

在代码库中识别出多处使用了 int/uint 而非 int256/uint256 的实例:

出于明确性考虑,考虑用 int256/uint256 替换所有实例的 int/uint

更新:pull request #8中的提交17c7c0a中解决。团队表示:

通过用 int256/uint256 替换所有实例的 int/uint,实施了审计员的建议。

缺少安全联系

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

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

更新:pull request #8中的提交7958ca9中解决。团队表示:

通过在每个合约定义上方添加包含安全联系的 NatSpec 注释,实施了审计员的建议。

重复导入

在代码库中识别出多处重复导入的实例:

考虑移除任何重复导入,以提高整个代码库的清晰度和可读性。

更新:pull request #9中的提交b8bd4e8中解决。团队表示:

通过删除代码库中各种接口的重复导入,实施了审计员的建议。

使用自定义错误

自 Solidity 版本 0.8.4 以来,自定义错误提供了一种更清晰、更经济的方式向用户解释操作失败的原因。

在整个代码库中识别出多个 revert 和/或 require 消息的实例:- 在 Beacon.sol2030

为了简洁性和节省 gas,考虑用自定义错误替换 requirerevert 消息。

更新:pull request #10 的提交 521ea7f 和提交 a80fbd3 中得到解决。团队表示:

通过在整个代码库中用自定义错误替换 requirerevert 消息,实现了审计人员的建议。

缺失文档字符串

在整个代码库中,发现多处缺失文档字符串的情况:

考虑全面记录所有公共 API 合约的所有函数(及其参数)。即使不公开的实现敏感功能的函数也应清楚记录。在编写文档字符串时,考虑遵循 Ethereum Natural Specification Format (NatSpec)。

更新:pull request #11 的提交 6c4e527 中得到解决。团队表示:

通过使用 natspec 添加缺失的文档字符串,实现了审计人员的建议。

不正确的文档字符串

在整个代码库中,发现多处文档字符串不正确的情况:

  • IERC721Soulbound 接口中的注释不正确,因为它们引用了标准的 ERC-721 代币合约,而不是预期的 soulbound 代币实现。
  • IERC721TokenInitIERC721SoulboundTokenInit 接口中的 initializeWithRoles 函数的注释不正确。具体来说,它们声称 ISSUER_ROLE 是用于发行新代币的角色,这具有误导性,并非事实。

考虑全面记录所有函数的公共 API。如果结构为公共 API 的函数的文档中有内容错误,请注意在编写文档字符串时,考虑遵循 Ethereum Natural Specification Format (NatSpec)。

更新:pull request #12 的提交 86ef758 中得到解决。团队表示:

通过用正确的 natspec 更新不正确的文档字符串,实现了审计人员的建议。

可预测的存储槽预影

合约使用 StorageSlot 库存储状态变量,该库将值存储在唯一的插槽中。这种方法的问题在于,对于所使用的 keccak256 而言,插槽具有已知的预影。如果使用 StorageSlot 库的合约在功能和复杂性上不断增长,并且包含允许攻击者写入其选择的存储槽的漏洞则会导致问题。

考虑从结果 keccak256 值中减去 1。

更新: 已确认,未解决。团队表示:

确认,我们理解这是对未来代码库/修改的合理预防措施,但是,如果我们升级现有实施合约以使用这种新槽计算,现有逻辑将不再访问之前存储的数据。除非我们手动迁移状态从旧槽到新槽,否则我们的代理合约将有效地“丢失”存储数据。尽管意图是使攻击者计算插槽的预影在计算上不可行,但在升级时这样做将在此时打破我们协议的向后兼容性。在我们当前的状态下,除了故意操作以外,现有逻辑不允许写入这些插槽,因此这一风险非常低。切换现有方案将是破坏性变化,高风险丢失状态数据,除非我们谨慎规划和执行存储迁移。

对于不存在代币无法设置 URL

ERC721BasicTokenERC721SoulboundToken 合约的 setTokenURI 函数不允许为不存在的 tokenId 设置 URI。这禁止使用这些合约处理一种常见用例,即在铸造相应代币之前设置 tokenId 元数据。

考虑取消此限制,以不限制 ERC721BasicTokenERC721SoulboundToken 合约的灵活性。

更新: 已确认,未解决。团队表示:

我们确认这一发现。此限制是有意的,这是我们在当前产品范围内不需要利用的功能,可以按需升级。

接口和合约之间的不一致

在整个代码库中,发现多处接口和实现合约之间的不一致:

考虑在接口和其实现合约之间对参数名称和存储位置关键字进行一致性对齐,以确保一致性并减少潜在错误。

更新:pull request #13 的提交 9ccc869 中得到解决。团队表示:

通过使接口中的参数名称和存储关键字与其实现合同保持一致,实现了审计人员的建议。

使用非显式导入

在代码中使用非显式导入可能会降低代码清晰度,并可能导致本地定义和导入变量之间的命名冲突。这在同一 Solidity 文件中存在多个合约或继承链较长时尤为相关。

在整个代码库中,发现多处非显式导入的情况:

出于代码更清晰的原则,考虑使用命名导入语法 ( import {A, B, C} from "X") 明确声明要导入的合约。

更新:pull request #14 的提交 906e1b2 和提交 32317ad 中得到解决。团队表示:

通过将导入转换为更显式的命名导入,实现了审计人员的建议。

未使用的 internal 函数

在整个代码库中,发现多处未使用的 internal 函数:

为了提高代码库的整体清晰度、目的性和可读性,考虑使用或删除当前未使用的函数。

更新: 已确认,未解决。团队表示:

_已确认。_increaseBalance_increaseBalanceVanila 函数是内部实用函数,属于 ERC721 代币实现模式的一部分。它们服务于特定目的:_increaseBalance 是一个安全包装程序,可防止在可枚举代币中批量铸造。 _increaseBalanceVanila 是实际的余额增加实现。这些函数实际上是通过 _update_updateVanila 函数间接使用的,这些函数在铸造和转移操作过程中被调用。但是,它们是通过与直接使用不同的代码路径调用的。safeMinttransferFrom 等公共函数调用 _update,而 _update 会调用 _updateVanila,后者使用与 _increaseBalanceVanila 相似的逻辑直接修改余额。这些函数存在的原因是为使用 ownerOf 重写方式“铸造”代币提供一个Hook点。它们当前在实现中没有被使用,但我们希望保留它们,因为它们为未来功能提供了扩展点。我们在 pull request #15 中为这些函数添加了额外的 natspec 说明。_

合约内部函数顺序不一致

在整个代码库中,发现多处合约内部函数顺序不一致的情况:

为了改善项目的整体可读性,考虑根据 Solidity Style Guide ( 函数的顺序 ) 的推荐在整个代码库中进行标准化排序。更新:已在 pull request #16 中解决,提交记录 6b471d6。团队声明:

通过与 Solidity 风格指南更紧密地重新排列函数,实施了审计员推荐的修复。

函数可见性过于宽松

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

为了更好地传达函数的意图并可能实现一些额外的 gas 节省,请考虑将函数的可见性仅更改为所需的宽松程度。

更新:已在 pull request #17 中解决,提交记录 e69bc2d。团队声明:

通过改变函数的可见性使其仅在代码库中按照需要宽松,实施了审计员推荐的修复。

不完整的文档字符串

在代码库中,发现了多个不完整的文档字符串实例:

请考虑充分记录所有函数/事件(及其参数或返回值),这些是合同公共 API的一部分。在编写文档字符串时,考虑遵循 以太坊自然规范格式 (NatSpec)。

更新:已在 pull request #18 中解决,提交记录 ec85574。团队声明:

通过使用 NatSpec 在代码库中添加更全面和完整的文档字符串,实施了审计员推荐的修复。

Gas 优化

在代码库中,发现了多个 gas 优化的机会:- upgradeBeaconToAndCall 函数在 ERC721BasicTokenERC721SoulboundTokenWhitelistComplianceOracle 合约中从 _BEACON_SLOT 存储中读取 beacon 地址,设置其值为 newBeacon 之后。这导致了不必要的存储读取。相反,可以直接使用 newBeacon 获取实现地址。

更新:pull request #19 的提交 554ed97 中解决。团队表示:

执行了审计师的建议,通过减少直接存储读取和在线圈中使用前缀递增运算符来应用 gas 优化。

结论

Whitelist Oracle 系统提供了一种合规框架,使得如 ERC-721 和 ERC-1155 等代币标准,以及外部 oracle 合约,能够验证转账。在审计过程中,识别出了一些小问题,并提出了提高代码一致性、可读性和 gas 效率的建议。

此外,在审计过程中,注意到代码库的分支覆盖率约为 58%,关键组件(ERC-721 Tokens 和 Whitelist Compliance Oracle)的覆盖率在 60% 至 68% 之间。建议加强测试套件并加入模糊测试,以增强代码库的成熟度。在审计之后,团队显著改善了测试套件,使核心组件的分支覆盖率达到 80-82%。WisdomTree Digital 团队在整个审计过程中积极合作,并提供了清晰的解释。

Request Audit

附录

问题分类

OpenZeppelin 将智能合约漏洞按 5 级进行分类:

  • 危急
  • 附注/信息

危急严重性

当问题的影响为灾难性,对客户的声誉造成重大损害和/或给客户或用户造成严重经济损失时,应用此分类。被利用的可能性可能很高, warranting a swift response。危急问题通常涉及重要风险,如永久性丢失或锁定大量用户的敏感资产,或核心系统功能在没有可行缓解措施的情况下失败。这些问题由于其可能严重损害系统完整性或用户信任,需立即关注。

高严重性

这些问题的特征是可能对客户的声誉产生重大影响和/或导致可观的财务损失。被利用的可能性显著, warranting a swift response。此类问题可能包括临时丢失或锁定大量用户的敏感资产,或对关键系统功能的干扰,尽管可能有一定但有限的缓解措施可用。重点在于对系统操作或资产安全产生显著但不总是灾难性的影响,需及时有效地补救。

中等严重性

被归类为中等严重性的问题可能对客户的声誉造成可注意的负面影响和/或适度的财务损失。此类问题如果得不到处理,具有适度被利用的可能性,或可能在系统中导致不希望的副作用。这些问题通常限于较小的一部分用户的敏感资产,或涉及偏离指定的系统设计,虽然这不直接与财务有关,但危害到了系统的完整性或用户体验。此处的重点在于那些造成真实但可控风险的问题,值得及时关注以防止升级。

低严重性

低严重性问题是对客户的运营和/或声誉产生低影响的问题。这些问题可能代表对客户具体商业模式的小风险或效率不足。它们被识别为可改进的领域,尽管不是紧急问题,但如果解决,会增强代码库的安全性和质量。

附注及其他信息严重性

此类别保留给那些尽管影响微乎其微,但仍然重要的问题。解决这些问题有助于整体安全状况和代码质量的改善,但不需要立即行动。这反映了对维持高标准和持续改进的承诺,即使是在不构成立即风险的领域。

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

0 条评论

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