Stellar合约库v0.3.0-rc.2审计

OpenZeppelin 对 Stellar Contracts Library v0.3.0-rc.3 版本进行了安全审计,发现了包括严重、高、中、低风险在内的37个问题,该版本引入了架构增强、新功能和扩展的开发者实用程序,特别是通证模块的重构和基于角色的访问控制机制。审计报告强调了潜在的安全风险,并提出了改进建议。

目录

总结

类型 DeFiTimeline 从 2025-06-04 到 2025-06-18 语言 Rust (Soroban) 总问题 37(32 已解决,2 部分解决)严重等级问题 1(1 已解决)高严重等级问题 1(1 已解决)中严重等级问题 5(4 已解决,1 部分解决)低严重等级问题 16(16 已解决)注释 & 附加信息 12(8 已解决,1 部分解决)客户端报告问题 2(2 已解决)

范围

OpenZeppelin 对 OpenZeppelin/stellar-contracts 仓库进行了差异审计,提交版本为 cf05a5d,对比提交版本 d3741c3

审计范围包括以下文件:

 packages
├── access
│   ├── access-control
│   │   └── src
│   │       ├── access_control.rs
│   │       ├── lib.rs
│   │       └── storage.rs
│   ├── access-control-macros
│   │   └── src
│   │       └── lib.rs
│   ├── ownable
│   │   └── src
│   │       ├── lib.rs
│   │       ├── ownable.rs
│   │       └── storage.rs
│   ├── ownable-macro
│   │   └── src
│   │       └── lib.rs
│   └── role-transfer
│       └── src
│           ├── lib.rs
│           └── storage.rs
├── constants
│   └── src
│       └── lib.rs
├── contract-utils
│   ├── crypto
│   │   └── src
│   │       ├── hashable.rs
│   │       ├── hasher.rs
│   │       ├── keccak.rs
│   │       ├── lib.rs
│   │       ├── merkle.rs
│   │       └── sha256.rs
│   ├── default-impl-macro
│   │   └── src
│   │       ├── helper.rs
│   │       └── lib.rs
│   ├── macro-helpers
│   │   └── src
│   │       └── lib.rs
│   ├── merkle-distributor
│   │   └── src
│   │       ├── lib.rs
│   │       ├── merkle_distributor.rs
│   │       └── storage.rs
│   ├── pausable
│   │   └── src
│   │       └── pausable.rs
│   ├── pausable-macros
│   │   └── src
│   │       └── lib.rs
│   ├── upgradeable
│   │   └── src
│   │       ├── lib.rs
│   │       ├── storage.rs
│   │       └── upgradeable.rs
│   └── upgradeable-macros
│       └── src
│           └── derive.rs
└── tokens
    ├── fungible
    │   └── src
    │       ├── extensions
    │       │   ├── allowlist
    │       │   │   ├── mod.rs
    │       │   │   └── storage.rs
    │       │   ├── blocklist
    │       │   │   ├── mod.rs
    │       │   │   └── storage.rs
    │       │   ├── burnable
    │       │   │   ├── mod.rs
    │       │   │   └── storage.rs
    │       │   ├── capped
    │       │   │   └── storage.rs
    │       │   └── mod.rs
    │       ├── fungible.rs
    │       ├── impl_token_interface_macro.rs
    │       ├── lib.rs
    │       ├── overrides.rs
    │       ├── storage.rs
    │       └── utils
    │           ├── mod.rs
    │           ├── sac_admin_generic
    │           │   ├── mod.rs
    │           │   └── storage.rs
    │           └── sac_admin_wrapper
    │               ├── mod.rs
    │               └── storage.rs
    └── non-fungible
        └── src
            ├── extensions
            │   ├── mod.rs
            │   └── royalties
            │       ├── mod.rs
            │       └── storage.rs
            ├── lib.rs
            ├── non_fungible.rs
            ├── overrides.rs
            └── storage.rs

更新:PR#330 合并后,本次报告中高亮显示的问题的修复已全部合并,提交版本为 857bd36

系统概述

Stellar Contracts Library 是一个模块化、可重用且安全的组件集合,用于在 Stellar 网络上构建智能合约。本次审计评估了该库的第三个候选发布版本(RC3),该版本引入了重大的架构增强、强大的新功能以及扩展的开发者实用工具集。与之前的版本相比,此版本侧重于通过更具凝聚力和可扩展性的架构来改善开发者体验,同时添加了备受期待的、可用于生产环境的功能。

Token标准(Token Standards)的架构改进

在 RC3 中,最显著的变化之一是对可替代和不可替代Token(NFT)模块进行的深度架构重构。以前的模型依赖于组合多个扩展 trait,现已被更统一和简化的模型所取代。

这种新架构以 Base 合约类型和 ContractOverrides trait 为中心。核心逻辑(如铸造和元数据处理)现在直接集成到 Base 实现中。这通过减少样板代码来简化标准Token的开发。对于更高级的用例,ContractOverrides 系统允许扩展(如新的 allowlistblocklist 模块)干净且安全地覆盖默认Token行为,例如 transferapprove,而无需开发者重写整个Token接口。

Stellar 资产合约(SAC)管理模块

SAC 管理模块允许用户与 SAC 集成。管理模块支持两种集成方法:通用方法和包装器方法。采用通用方法,sac-admin-generic 模块利用 __check_auth 函数来处理身份验证和授权,从而可以注入自定义授权逻辑。相反,sac-admin-wrapper 模块使管理模块充当中间件,为每个管理函数定义特定的入口点,并将这些调用转发到相应的 SAC 函数。这允许在委托之前应用自定义逻辑,从而实现模块化和直接的设计。

访问控制框架

此版本引入了一个全面而灵活的框架,用于管理链上权限,该框架由两个不同的模块组成:

  • ownable 一个用于实现简单的单所有者访问控制的模块。它为需要所有者通过 #[only_owner] 宏授权的函数提供了一个简单的模式。
  • access-control 一个功能齐全的基于角色的访问控制(RBAC)系统,用于需要更精细权限的合约。它支持创建自定义角色(例如 mintermanager 等)、将角色分配给多个帐户,并通过将某些角色指定为其他角色的管理员来建立管理层次结构。

这两个模块都具有安全的两步转移机制,用于主要的 owneradmin 角色。这种设计选择通过要求提议的新管理员在转移完成之前明确接受该角色来防止意外失去控制。

开发者体验和宏

此版本的一个主要主题是开发者体验的显著改善,这主要得益于强大的过程宏的引入和改进。这些宏旨在减少样板代码,强制执行一致的安全模式,并自动化常见的实现任务,从而使该库更易于访问和更强大。

  • 授权宏: 一套新的属性宏提供了一种干净且声明性的方式来实现访问控制:

    • #[only_owner]:与 ownable 模块一起使用,此宏将函数执行限制为单个指定的合约所有者。
    • #[only_admin]:与 access-control 模块一起使用,此宏将函数执行限制为顶级合约管理员。
    • #[has_role(...)]:一个灵活的宏,用于 access-control 模块,用于检查指定帐户是否拥有给定角色(例如,#[has_role(caller, "minter")])。这使开发者可以轻松地为不同的合约函数创建和强制执行细粒度的、基于角色的权限。
  • 默认实现宏(#[default_impl]): 此现有宏已在架构上得到增强,以进一步简化开发。它现在与核心Token trait 上的 type ContractType 关联类型一起运行。这允许开发者简单地指定所需的基类行为(例如,type ContractType = Base;),并让宏自动生成完整的、符合标准的接口。此更改大大减少了开发者需要为标准Token实现编写的代码量,并且已扩展为支持新的 AccessControlOwnable trait 。

  • 可升级合约宏:现有的 #[derive(Upgradeable)]#[derive(UpgradeableMigratable)] 宏已使用一项关键的新功能进行了更新:它们现在会在编译期间自动读取 CARGO_PKG_VERSION 环境变量,并将此版本字符串作为 binver 元数据嵌入到合约的 Wasm 中。这将创建一个不可变的、链上的合约版本记录,为链下工具和升级管理提供关键信息。

链上 NFT 版税

NFT 模块中添加了一个新的 royalties 扩展,提供了一个标准化的链上版税支付机制,该机制符合 ERC-2981 标准。这允许创建者和开发者以编程方式直接从他们的 NFT 合约中强制执行版税经济。

该扩展支持:

  • 整个系列的默认版税,除非被覆盖,否则适用于所有Token。
  • 特定于Token的版税,可以在铸造时设置,以定义单个 NFT 的唯一版税条款。

Merkle 树和密码学实用工具

为了支持更高级的用例(如空投和链下投票),RC3 引入了一组通用的密码学实用工具:

  • 一个 crypto 包提供了通用的哈希原语(Sha256Keccak256 等)和一个用于 Merkle 证明的通用验证器。
  • 一个 merkle-distributor 模块利用这些原语来为构建通过 Merkle 证明在链上验证声明的应用程序提供一个可重用的模板。

这些实用工具在新的示例合约中展示,包括 fungible-merkle-airdropmerkle-voting,这些合约为开发者提供了实现这些模式的实用模板。

其他Token扩展和示例

除了上面讨论的主要功能外,此版本还包括:

  • 用于可替代Token的新 allowlistblocklist 扩展,使合约管理员可以精细地控制允许哪些帐户与Token进行交互。
  • 新的示例演示了为 SAC 创建自定义管理员的模式,利用新的 access-control 框架来管理 SAC 的特权函数。

安全模型和信任假设

使用此库构建的任何合约的安全性都取决于库本身的正确性和关于其环境和使用的一组信任假设。

  • 范围外的依赖项: 本次审计假设底层 Soroban 环境、目标 WASM 编译器、外部 crate 和 Soroban SDK 的安全性和正确功能。这些组件构成了合约运行的可信基础,但不属于本次审查的直接范围。

  • 开发者责任: Stellar Contracts Library 提供了一组强大但无偏见的原始类型。它不强制执行特定的安全模型,而是为开发者提供构建自己的安全模型的工具。因此,智能合约的最终安全性关键取决于开发者对其的实现。关键责任包括:

    • 实施授权: 低级函数(如 Base::mint)在设计时故意没有内置访问控制。因此,开发者必须将对这些函数的调用包装在强制执行适当授权的函数中,例如,使用提供的 #[only_owner]#[has_role(...)] 宏。否则将导致特权函数可公开调用。
    • 安全初始化: 合约的 owneradmin 必须在构造函数中安全设置。未正确配置的合约可能没有管理员,从而使其无法管理。
    • 跨合约调用授权 由于 Stellar 网络的授权模型,当合约地址是函数的调用者时,require_auth() 检查会自动通过。因此,假设如果一个合约地址被分配了一个特权角色,则调用合约中会执行适当的授权检查。
  • 链下组件: 依赖于链下过程的功能(如 merkle-distributor)取决于在区块链外部安全且正确地生成数据。虽然本次审计涵盖了链上证明验证逻辑,但它假设提供给合约的 Merkle 根已正确生成并受到信任。

严重等级: критичecкий

由于 u32 约束,版税逻辑不适用于典型的 NFT 市场价值

royalty_info 函数旨在通过返回基于给定 sale_price 和基点(basis points)的应付版税金额来实现 EIP-2981 版税计算。该函数使用 (sale_price * basis_points) / 10000 公式来计算付款。但是,对 sale_price 输入使用 u32 类型施加了重大的实际限制。例如,在 Stellar 上,USDC 有 7 个小数位(1 USDC = 10,000,000),u32 可以容纳的最大值(4,294,967,295)大致相当于 429 USDC。

此约束使该函数无法用于高价值销售或具有高小数精度的Token,这在 NFT 市场中很常见。因此,不能使用高于此阈值的任何销售价格。此限制直接破坏了版税系统的预期功能,尤其影响依赖二级销售版税的艺术家和创作者。根据当前的市场动态,NFT 销售额通常超过数千个 USDC,使得 u32 对于实际应用来说严重不足。

考虑将 sale_price 和版税返回类型更改为 i128,它提供了足够大的范围来适应实际的销售金额,并确保准确的版税分配,而不会牺牲精度。

更新: 已在 pull request #290commit ad09aacpull request #329commit 68397ff 中解决。

严重等级: высокий

#[has_role] 宏提供的收益极小,并引入了授权风险

#[has_role(account, "role")] 过程宏通过将身份验证(require_auth())与授权(ensure_role)分离来引入安全风险。它验证任意 Address 是否拥有角色,但不验证此地址的签名。这种分离可能导致严重的漏洞,从而使代码能够成功编译,但违反了核心访问控制原则。

该宏的设计营造了一种虚假的安全感。开发者,尤其是来自像 Solidity 这样的生态系统的开发者,习惯于对调用者进行身份验证的基于角色的修饰符。当前的宏鼓励不正确的假设,从而导致不安全的实现。例如:

 #[has_role(minter_account, "minter")]
pub fn mint_for_anyone(e: &Env, minter_account: Address, recipient: Address) {
    // Missing minter_account.require_auth() allows an attacker to
    // impersonate a valid minter by simply passing their address.
    internal_mint(e, &recipient);
}

即使正确使用,该模式也很混乱,并迫使开发者和审计员仔细验证宏中的变量是否与签名中的变量匹配,并且 require_auth() 调用是否确实存在于该变量中。函数的安全性取决于一个容易遗漏的代码行。这增加了开发和审查期间出错的可能性。此外,该宏提供的符合人体工程学的价值极小。与直接调用 ensure_role 相比,它只节省了一行代码,同时引入了很大的潜在误用。这种微弱的权衡破坏了它的效用。

考虑引入一个更安全且对开发者友好的宏,如 #[only_role(caller, "role")](类似于 Solidity 的 OpenZeppelin 库中的流行修饰符),它将自动注入身份验证和授权检查。这种方法符合预期,消除了歧义,并默认强制执行安全性,从而真正利用宏的力量来减少样板代码,而不会损害安全性。

更新: 已在 pull request #318commit b303e42 中解决。

严重等级: средний

在可替代(Fungible)的黑名单(BlockList)扩展中缺少对 Spender 的检查

blocklist 扩展的 transfer_from 函数不验证 spender 是否在黑名单中。因此,黑名单中的地址仍然可以代表非黑名单用户执行 transfer_from 操作,前提是它已通过 approve 预先批准。这与 BlockList trait 相矛盾,该 trait 指定 被阻止的帐户无法转移Token。

这可能会导致混淆,即 from 帐户可能认为黑名单中的 spender 不允许与智能合约交互,尽管该 spender 仍然可以从预先批准的 allowance 中转移Token,从而破坏了黑名单机制的完整性。一个实际的漏洞利用场景可能涉及一个已获得用户批准的受损合约。即使在被列入黑名单之后,该合约仍将能够通过 transfer_from 耗尽用户资金,从而绕过黑名单系统的预期保护。

transfer_from 函数中,考虑添加一项检查,以确保 fromspender 地址未被列入黑名单。

更新: 已部分解决。该合约仍然允许黑名单中的 spender 与预先批准的Token进行交互。但是,已添加文档以警告 pull request #307commit 14813be 中的此行为。OpenZeppelin Stellar 开发团队表示: 这是遵循以太坊等其他生态系统的惯例所做的有意选择。

缺少放弃管理员(Admin)的函数

access-control trait 没有提供管理员放弃其角色的方法。这是有问题的,因为 Admin 密钥存储在实例存储中,并且即使不再使用,在 TTL 扩展期间仍将继续被考虑在内,从而增加了操作成本。此外,如果没有放弃机制,就无法有意地使合约永久性地没有管理员,这通常是出于去中心化或升级目的而希望的。

考虑添加一个 renounce_admin 函数,允许当前管理员从角色中删除自己。

更新: 已在 pull request #316commit 0b8dbbd 中解决。

当未设置管理员时,ensure_if_admin_or_admin_role 会发生 panic,并忽略角色管理员

ensure_if_admin_or_admin_role 函数 旨在授权作为合约管理员或特定角色管理员的调用者。但是,当前的实现首先检查合约管理员并调用 get_admin,如果 Admin 密钥不存在于存储中,则会发生 AdminNotSet 错误。这会阻止该函数继续检查调用者是否是角色管理员,从而有效地禁用对没有合约范围管理员的合约中敏感的基于角色的函数(如 grant_rolerevoke_role)的访问。

AccessControl trait 声明 一个合约必须 setting 管理员才能够使用该 trait 的功能。但是,鉴于这是一个库,它应该提供足够的灵活性,以允许在没有管理员的合约中设置角色。想象一个场景,其中管理员分配所需的角色和角色管理员,然后放弃其权力。在这种情况下,相应的角色管理员应该能够控制对其各自角色的访问。

考虑处理 get_adminensure_if_admin_or_admin_role 函数中返回的 AdminNotSet 错误,并确保及时扩展 Admin 密钥的 TTL。

更新: 已在 pull request #292commit 132ecab 中解决。

不一致的实例存储 TTL 扩展

在整个代码库中,实例存储的 TTL 没有在任何地方扩展,除了 ownable trait 的 get_owner 函数。这在 TTL 扩展的处理方式中产生了一种不一致的模式。与其他存储类型不同,扩展实例存储的 TTL 会影响整个合约存储,并且成本可能很高。同时,未在需要时扩展 TTL 可能会导致关键功能丢失,并可能在 TTL 过期后使整个智能合约无法访问。因此,重要的是要明确 TTL 扩展应该在库中处理还是由集成商显式处理。

考虑在整个代码库中扩展实例 TTL,或删除 get_owner 函数中的扩展,并明确记录 TTL 扩展是集成商的责任。

更新: 已在 pull request #293commit 4775033pull request #329commit 68397ff 中解决。

AllowListBlockList 扩展上缺少 FungibleBurnable 实现

FungibleBurnable trait 提供了两个关键函数:burnburn_from。这些是 SEP-0041 Token接口的一部分,应该可用于任何可替代Token扩展。目前,这些函数已在 Base 合约类型上实现。但是,当尝试将 FungibleBurnable 接口与其他扩展(如 AllowListBlockList)一起使用时,由于缺少 trait 在这些特定类型上的实现,因此找不到预期的方法。这会阻止用户使用 default_impl macro 生成默认实现,其中 contractType 关联类型指定为 Base 类型以外的其他类型。

考虑将 burnburn_from 函数添加到 AllowListBlockList 实现。 更新:已在pull request #294commit be401b0中解决。

低严重性

无法移除 Token 特定的版税

NFT 版税扩展提供了定义默认的、集合范围内的版税和覆盖默认值的 token 特定版税的功能。set_token_royalty 函数允许为单个 token 设置版税,而 set_default_royalty 建立集合级别的默认值。但是,该实现缺少一种在设置后删除 token 特定版税的机制。

这在系统中造成了刚性:具有自定义版税的 token 无法恢复为使用集合默认值。模仿默认值需要显式地在 token 特定条目中存储相同的值,这既是冗余的,也是低效的。royalty_info 函数始终优先考虑 token 特定设置而非默认设置,这意味着一旦单独配置了一个 token,它就会永久地与集合默认值的未来更改隔离。

在实践中,NFT 集合通常需要随着时间的推移调整版税策略,例如,降低版税以增加交易量,或由于组织更新而更改接收者。如果没有办法删除 token 特定的覆盖,这些 token 就会与不断演变的集合策略脱节。这导致了碎片化的版税结构并限制了灵活性。

考虑引入一个 remove_token_royalty 函数,该函数显式地从持久存储中删除 token 特定的版税 key,允许 token 回退到集合范围的默认值。

更新:已在pull request #296commit 4a27a5e中解决。

宏接受并静默忽略参数

代码库中的几个过程宏接受并静默忽略通过 TokenStream 传递的参数,这可能会导致混淆和误用。例如,像 #[only_admin(caller)] 这样的宏在语法上接受 caller 参数,但实现完全忽略它。

具体来说,以下宏会忽略它们的参数输入:

通过接受然后静默地丢弃它们的参数,这些宏给人一种错误的自定义印象:用户可能认为他们正在配置行为(无论是访问检查还是默认实现),而实际上,什么也没发生。外观和操作之间的这种不匹配不仅混淆了代码的意图并使审查复杂化,而且还为不良行为者提供了一个简单的途径来插入欺骗性的注释,这些注释掩盖了未经授权的逻辑或意外的默认值。

为了提高清晰度和开发者体验,请考虑在上述宏中显式地拒绝未使用的参数。这可以通过检查 attr.is_empty() 并使用如下消息触发编译时 panic 来完成:

 assert!(attr.is_empty(), "This macro does not accept any arguments");

这将有助于防止误导性使用并提高基于宏的抽象的可靠性和透明度。

更新:已在pull request #295commit 966775epull request #327commit 110f6ce中解决。

Merkle 分发器中不受限制的证明大小和叶子索引

merkle-distributor 模块的 set_root 函数允许只设置一次 root,表明该树具有静态大小(假设为 nnn)。因此,最多应该有 nnn 个叶子,因此,最多允许 nnn 个 claim。对于一个完整的二叉 Merkle 树,高度应该是 log₂(n)log₂(n)log₂(n)。但是,当调用 verify_and_set_claimed 函数时,没有验证 proof 的长度(以确保它与树的高度匹配)或 index 值(以确认它小于 nnn)。这种疏忽可能允许攻击者伪造一个长度不正确的证明,该证明可能比树的实际高度短或长。

由于树是静态的,具有预定的大小,因此考虑将树的大小(或最大叶子数)作为一个变量与 root 一起包含。这将允许限制 proof 长度和叶子 index,从而提高安全性,防止潜在的证明伪造。

更新:已在pull request #322commit 50cd251中解决。

缺少检索所有角色成员的函数

access-control 模块通过 get_role_member(role, index)get_role_member_count(role) 实现访问控制,允许枚举角色成员。但是,它缺少一个 single-call getter,该 getter 返回给定角色的所有成员,如 OpenZeppelin 的 AccessControlEnumerable 合约的 getRoleMembers(bytes32 role) 函数所提供的那样。

虽然可以理解的是,该合约省略了此函数以避免可能耗尽资源限制的无界调用,但缺少批量访问器可能会使 off-chain 集成复杂化。客户端需要手动迭代从 0 到 get_role_member_count(role) - 1 的索引,以重建完整的角色成员列表,这会增加额外的复杂性以及潜在的集成错误。

考虑添加一个函数,该函数返回给定角色的完整成员集,并附带警告大型集合可能存在资源限制问题的文档。如果此功能被有意省略,显式地记录此设计选择将有助于指导开发者。

更新:已在pull request #311commit 2bd717apull request #330commit 41060f2中解决。

如果 live_until_ledger 超过临时条目的最大 TTL,则 transfer_role 会 panic

transfer_role 函数允许设置一个 live_until_ledger 值来指定临时条目的过期时间。但是,如果提供的 live_until_ledger 隐含的 TTL 超过了临时条目的最大限制,该函数将会 panic。这种行为与持久条目的处理方式不同,在持久条目中,TTLs 被限制为最大值,而不是导致 panic。这可能会导致意外的失败,特别是因为调用者可能合理地期望该函数能够优雅地处理这种情况。在这种情况下,panic 对用户不友好。

考虑在使用 live_until_ledger 输入之前对其进行验证,或者将 TTL 限制为临时条目的最大允许值,从而匹配持久条目的行为。此外,应清楚地记录此 edge case,以防止误用并确保可预测的合约行为。

更新:已在pull request #298commit ccad351中解决。

缺少验证

set_adminset_owner 函数被设计为仅在智能合约的生命周期内调用一次。这些角色的任何进一步设置都应分别通过 transfer_admin_roletransfer_ownership 函数完成。但是,没有验证可以防止 setter 函数被多次调用。

考虑添加一个检查,以使用 e.storage().instance().has(&key) 验证相应的 key 是否已设置,如果存在则使用适当的错误(例如,AdminAlreadySetOwnerAlreadySet)回退并出现 panic。

更新:已在pull request #299commit 16bdda0中解决。

核心合约的误导性命名可能会让熟悉 openzeppelin-contracts 的开发者感到困惑

库中的几个合约使用在 EVM 生态系统中被广泛认可的名称,例如 OwnableAccessControl,但实现了通常预期的逻辑的变体。例如,名为 Ownable 的合约实现了两步所有权转移模式,该模式更接近 OpenZeppelin EVM 合约库中的 Ownable2Step。类似地,AccessControl 的实现偏离了标准的 EVM 实现。

鉴于该库正在 OpenZeppelin 品牌下为 Stellar 生态系统开发,从 EVM 环境过渡的用户可能会基于熟悉的合约名称假设相同的语义和使用模式。如果开发者依赖于这些合约中不存在的隐式行为,这可能会导致关于功能的不正确假设、不正确的集成,甚至安全漏洞。

考虑重命名这些合约以更准确地反映它们的行为。或者,考虑提供清晰的文档免责声明和命名说明,以帮助开发者正确理解这些区别。

更新:已在pull request #328commit e47d949pull request #330commit 41060f2中解决。

缺少不安全函数的文档

set_rootset_claimed 函数都可以执行敏感的状态更新,而无需任何授权。从示例用法来看,set_root 应该只在构造函数中使用,而 set_claimed 可以是一个私有函数。

对于不安全的函数,考虑添加文档来演示它们的安全用法,并警告它们缺少授权。

更新:已在pull request #322commit 50cd251中解决。

Merkle 树验证可以更通用

crypto 模块只支持使用交换哈希函数生成的自定义 Merkle 树。查看文档,这似乎可能是由 OpenZeppelin/merkle-tree JaveScript 库树生成方案驱动的,因此当只使用 keccak256 哈希函数来哈希叶子时会发出警告。考虑到 OpenZeppelin/merkle-tree 库为成员包含证明生成 Merkle 树,该证明对 abi.encode ed 底层值的叶子使用双 keccak256 哈希函数,对内部节点使用排序的哈希,它更适合 Ethereum 上下文。

在 Stellar 网络上,编码方案是 XDR,并且 sha256 哈希函数似乎使用更广泛。因此,开发者不太可能使用适应 Ethereum 上下文的树。对 Merkle 树验证的有限支持仅限于交换哈希,这阻碍了更广泛的开发者采用,因为任何哈希函数的 多数直接应用,无论是 sha256 还是 keccak256,都不是可交换的。此外,链上累积构建的树自然对顺序敏感,并且在树生成中对排序然后进行哈希也需要更多的计算。

考虑支持更简单且限制性更小的普通哈希方案,而无需首先排序,并提供更适合 Stellar 网络的相关文档。

更新:已在pull request #321commit 79af2d9中解决。

has_role 宏中支持多个角色

当前的 has_role 宏仅验证单个角色,如果调用者缺少该角色,则会回退,迫使开发者在多个角色授予相同权限的情况下复制逻辑。这增加了样板代码并模糊了预期的访问策略。允许 has_role 接受多个角色将允许指定一个角色数组,并在调用者拥有其中任何一个角色时立即授予访问权限。此更改提高了清晰度并减少了重复代码。

考虑扩展 has_role 宏的签名,使其接受角色列表,迭代每个角色,并且如果任何检查通过则允许执行,同时保持对单角色检查的向后兼容性。

更新:已在pull request #325commit e7251be中解决。

潜在的循环管理员

access-control 模块中,可以以循环方式分配 RoleAdmin。例如,可以将 MINT_ADMIN 分配为 MINT_ROLE 的管理员,同时让 MINT_ROLE 成为 MINT_ADMIN 的管理员。但是,当发生这种情况时,可能会产生意想不到的后果。例如,当一个角色是另一个角色的管理员时,可能会创建一个角色撤销另一个角色的竞争条件。

由于 set_role_admin_no_auth 是一个受限制的函数,如果无法在代码中阻止上述行为,请考虑警告上述行为。

更新:已在pull request #312commit e0a8118中解决。

次优的存储和 TTL 策略

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

为了避免产生不必要的成本,请考虑添加 if 检查以仅在新状态与当前状态不同时才执行存储写入(并发出事件)。此外,当撤销访问权限或解除阻止用户时,请考虑使用以下代码删除条目,而不是写入 false

 e.storage().persistent().remove(&key);

更新:已在pull request #303commit bf51ae5中解决。

代码重复

代码重复会导致冗余操作并浪费计算成本。

grant_role_no_auth 函数中,AccessControlStorageKey::HasRole(account, role) 密钥首先在 add_to_role_enumeration 函数中设置,然后在 grant_role_no_auth 函数中再次设置。

考虑删除任何重复代码的实例。

更新:已在pull request #304commit 7158c50中解决。

可以删除未使用的 AccessControlStorageKeys

可以使用 set_role_admin 函数中的相同角色密钥设置和重置 AccessControlStorageKeys.RoleAdmin(RoleSymbol)。但是,对于不再使用的角色密钥,无法将其删除。每次调用 get_role_admin 函数时,都可以延长其 TTL,从而增加调用者的成本。

类似地,当 AccessControlStorageKeys.RoleAccountsCount(RoleSymbol) 为零时(例如,当角色符号不再使用时),无法删除 ledger 密钥。因此,当调用 get_role_member_count 函数时,可以延长 TTL,从而增加调用者的成本。

考虑允许管理员删除不再使用的角色的未使用密钥。

_更新:已在pull request #306commit d9d3ab5中解决。该 pull request 引入了 remove_role_admin_no_authremove_role_accounts_count_no_auth 函数,并警告这些函数不实现任何授权。_

transfer_role 仅使用 TTL 扩展策略可能会超过预期的过期时间

transfer_role 的当前文档表明,设置 live_until_ledger 将导致挂起的角色entry在 ledger 处完全过期。实际上,Soroban 的 extend_ttl 只能在entry的剩余 TTL 低于给定阈值时增加entry的 TTL。它不能缩短或重置更大、默认的 TTL。因此,当 live_for = live_until_ledger – current_ledger 小于临时 entry 的默认 TTL 时,调用

 e.storage().temporary().extend_ttl(pending_key, live_for, live_for);

没有效果。然后,该entry将保持活动状态,直到其原始 TTL 流逝,而不是在 live_until_ledger 处,从而导致潜在的用户困惑。

为了纠正这种误导性文档,请考虑更新函数的注释以明确警告:

  • live_until_ledger 是一个上限,而不是保证的到期时间
  • Soroban 的 TTL 策略 仅扩展 密钥的 TTL,不能减少 现有的 TTL
  • 如果计算出的 live_for 短于默认的最小 TTL,则该entry将比 live_until_ledger 存活得更久

在文档中包含这些说明 将有助于确保开发者理解实际到期可能超过指定的 ledger。

更新:已在pull request #323commit 72053cb中解决。

具有误导性和不准确性的文档

在整个代码库中,发现了多个具有误导性和/或不准确性的文档实例:

  1. [FungibleAllowList trait的文档](https://github.com/OpenZeppelin/stellar-contracts/blob/cf05a5d522f323274939a8bfbcea706fe21eeb52/packages/tokens/fungible/src/extensions/allowlist/mod.rs#L不同于项目中其他将实现(`src/*.rs`)与测试(`src/test.rs`)分离的库,`crypto特性比如hashable.rs` 将 公共函数测试 放在同一个文件中。

为了提高代码库的一致性和清晰度,考虑通过将测试移动到单独的文件中,使 crypto 目录与整体结构保持一致。

更新:已在 pull request #321commit 79af2d9 中解决。

Panic 处理中的不一致性

代码库在处理与缺失密钥和授权失败相关的 panic 时存在不一致性。

例如,get_admin 函数从实例存储中检索 Admin 密钥,如果未找到该密钥,则 直接 panic。此函数在 enforce_admin_auth 中使用,如果在授权失败时执行的是隐式 panic,而没有显示有意义或结构化的错误。相比之下,如果缺少 Owner 密钥,get_owner 函数返回 None。当 enforce_owner_auth 中使用时,它会检查授权并 panic 并显示 NotAuthorized 错误。但是,如果实际问题是所有者密钥不存在,则此错误可能会产生误导。

考虑通过使用带有清晰且不同的错误消息的 panic_with_error! 来标准化缺失密钥和失败授权的处理。

更新:已在 pull request #326commit 9a396fe 中解决。

潜在增加的常量

目前,所有 TTL 阈值和扩展量值都定义在一个文件中,即 constants/src/lib.rs。虽然这目前有效,但随着代码库和库数量的增长,这种中心化方法可能会变得更难以维护。在一个位置管理所有常量可能会导致可读性问题。

考虑通过在每个相关目录或模块级别引入专用的 constants.rs 文件来模块化 constants/src/lib.rs 中的常量。随着项目的发展,这将促进更好的组织、封装和更易于维护。

更新:已确认,将会解决。OpenZeppelin Stellar 开发团队表示:

这是一个很好的建议。但是我们计划在项目发展壮大时再这样做。目前,这将给项目增加不必要的复杂性。它现在这样更简洁。

管理员角色转移缺少强制延迟

access-control 模块的 transfer_admin_role 函数包含一个 live_until_ledger 参数,用于限制可以接受转移的时间窗口,但它没有强制在转移的启动和接受之间设置强制延迟。因此,新管理员可以在下一个账本中接受该角色,从而有效地实现即时角色转移。

这与 OpenZeppelin AccessControlDefaultAdminRules 合约不同,后者强制在启动(beginDefaultAdminTransfer)和接受(acceptDefaultAdminTransfer)之间设置最短延迟。该延迟是对抗恶意或受损管理员转移的关键保障,它使利益相关者有时间检测可疑活动并协调响应。

如果没有强制延迟,系统很容易受到快速接管的影响,从而没有时间进行社区审查、链下协调或紧急干预。在任何人注意到之前,恶意提案或错误转移可能会最终确定。时间锁机制将提供低摩擦、高价值的保护层。它可以争取时间让代币持有者、集成商和治理参与者做出反应、暂停合约或发出危险信号。它还让当前的管理员有机会取消或回滚转移(如果在启动后出现新的问题)。

为了与安全治理工具中的最佳实践保持一致,请考虑在管理员转移的启动和接受之间添加最短强制延迟。

更新:已确认,但未解决。OpenZeppelin Stellar 开发团队表示:

延迟功能确实提供了另一层安全保障,但预计类似的功能将在专用的时间锁合约中开发,该合约可用于实现类似的结果。为此特定模块单独开发延迟似乎是不必要的重复工作。

易受攻击的依赖项

运行 cargo audit 发现 sac-admin-generic-example 0.3.0 使用的传递依赖项中存在两个已知的漏洞,即 curve25519-dalek 3.2.0ed25519-dalek 1.0.1

虽然上述依赖项仅在示例中使用过,但请考虑将其升级到最新的、非易受攻击版本。或者,考虑重构或删除受影响的示例代码,以消除对易受攻击 crate 的依赖。

更新:已在 pull request #319commit a619477 中解决。

更改 SAC 管理员地址时缺少警告

Stellar 文档 明确警告说,在更新 SAC 管理员地址时,不会在更改期间验证新的管理员地址。因此,可能会无意中分配无效或不正确的管理员地址,这可能会不可逆转地锁定 SAC 的管理控制。

虽然这对于任何管理智能合约都是如此,但请考虑将此警告添加到 SAC 管理员相关的库文档中,以反映风险并与 Stellar 文档保持一致。

更新:已在 pull request #320commit 873a416 中解决。

拼写错误

在整个代码库中,发现了多个拼写错误:

  • access-control/src/access_control.rs第 64 行 中,“has”应为“have”。
  • access-control/src/lib.rs第 44 行 中,“to with”应为“to go with”。
  • default-impl-macro/src/lib.rs第 22 行 中,“macro's”应为“macros”。

考虑更正所有拼写错误,以提高代码库的清晰度和可读性。

更新:已在 pull request #308commit dab8fc5 中解决。

重载的错误掩盖了不同的失败情况

某些错误的当前实现将多个语义上不同的失败场景合并为一个通用错误,从而大大降低了开发人员的清晰度和故障排除能力。尽管发生在不同的上下文中,但错误消息保持不变,没有提供有关失败根本原因的任何信息:

  • AccessControlError::AccountNotFound 错误用于指示各种不相关的问题。在 get_role_member 函数中,当提供的索引超出角色的成员列表的范围时,会触发错误,更合适的错误是 IndexOutOfBounds 之类的描述性错误。在 revoke_rolerenounce_role 函数中,当目标帐户没有指定的角色时,会发生错误,最好表示为 RoleNotHeld。在 remove_from_role_enumeration 函数中,错误可能源于角色没有成员或指定的帐户不是角色的一部分,最好用不同的错误(例如 RoleIsEmptyRoleNotHeld)来更准确地捕获该错误。

  • AccessControlError::Unauthorized 错误在 ensure_if_admin_or_admin_role 函数中使用,以指示调用者既不是合约管理员也不是角色管理员,但是,当未设置角色管理员时也会抛出此错误,最好用诸如 RoleAdminNotHeld 之类的错误来更准确地捕获该错误。

  • 当将不存在的角色与 has_role 宏一起使用时,将返回 AccessControlError::Unauthorized 错误消息,该错误消息与帐户没有现有角色时返回的错误消息相同。

考虑重构 AccessControlError 枚举以包含更精细的变体,以反映正确的错误消息。此更改可为开发人员提供即时、与上下文相关的反馈,并提高合约的可维护性。

更新:已在 pull request #309commit 86d038d 中解决。

transfer_role 中的边缘情况导致 NoPendingTransfer

live_for计算live_until_ledgercurrent_ledger 之间的差。如果 live_until_ledgercurrent_ledger 值相等,则 live_for 变为零。因此,pending_key 已设置,但 TTL 未扩展,从而导致空操作并浪费计算资源。转移的接受将仅限于当前的账本,这可能是一个非常短的时间范围,并且如果再次启动转移,将产生额外的成本。

考虑在尝试 TTL 扩展之前检查 live_for 是否满足最小阈值,并确保预期的 TTL 持续时间对于实际使用来说足够长。

更新:已确认,但未解决。OpenZeppelin Stellar 开发团队表示:

为了涵盖边缘情况,建议是进行额外的检查,这将使一般用例的成本更高。我们只是不认为这种方法的优点超过了使一般用例恶化的缺点。

命名建议

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

  • validate_param_type 函数可以重命名为 validate_address_type,因为它仅验证 Address 类型。
  • 在函数名称中使用 unsafe 以更明确地说明危险。这是 Rust 中不安全操作的常见模式:

    • grant_role_no_auth -> unsafe_grant_role
    • revoke_role_no_auth -> unsafe_revoke_role
    • set_role_admin_no_auth -> unsafe_set_role_admin
  • 在 Merkle 树上下文中,node 通常指内部节点,而 leaf 用于指树的成员。因此,IndexableNode 可以更准确地重命名为 IndexableLeaf。同样,node 函数参数可以命名为 leaf,局部变量 leaf 可以命名为 leafHash 以区别于参数。

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

更新:pull request #313commit f0a4daepull request #322commit 50cd251 中部分解决。OpenZeppelin Stellar 开发团队表示:

我们已接受了 3 项建议中的 2 项。但是,我们不会用 unsafe 替换 no_auth。这是因为在 Rust 中,unsafe 具有不同的含义。这意味着我们为了更大的灵活性和优化而选择退出 Rust 编译器的安全保证。我们的 no_auth 函数并非如此。

未使用的返回值

enforce_admin_authenforce_owner_auth 函数分别返回 adminowner 值。但是,这些值未在 onlyAdminonlyOwner 宏的上下文中被使用,从而使宏的使用成本略高。enforce_owner_authtransfer_ownership 函数 中被调用,其中 owner 返回值正在被使用。相反,transfer_admin_role 调用 get_adminadmin.require_auth() 而不是调用 enforce_admin_auth 函数。采用一致的方法来使用这些返回值将提高代码的可读性。

为了提高代码库的一致性和可维护性,请考虑在 transfer_admin_role 函数中使用返回值或完全删除它们。

更新:已在 pull request #315commit a954d5d 中解决。

在放弃所有权时进行不必要的 TTL 扩展

renounce_ownership 函数中,临时存储中的 PendingOwner 密钥会 被检查,以确定所有权转移是否正在进行中。如果密钥存在,则 TTL 会被扩展,并且在此之后,该函数会 立即恢复,表明如果所有权转移正在进行中,则不应允许放弃所有权。鉴于该函数在 TTL 扩展后立即 panic,因此扩展会被回滚并且不起作用,从而导致计算成本浪费。

考虑从 renounce_ownership 函数中删除 TTL 扩展。

更新:已在 pull request #314commit a15bf50 中解决。

客户报告

版税中缺少事件

non-fungible 特性的 royalties 扩展 在设置版税时不会发出任何事件。

考虑在与版税相关的状态更改时发出事件。

更新:已在 pull request #251commit 4267970 中解决。

缺少默认实现

一些支持的特性没有通过 default_impl macro 进行默认实现。此类特性包括 FungibleAllowListFungibleBlockListNonFungibleRoyalties

考虑包含所有支持的特性的默认实现。

更新:已在 pull request #252commit e7d8120 中解决。

结论

经审计的代码库引入了 Stellar Contracts Library 的 Release Candidate 3 (RC3),该版本带来了重要的架构改进、增强的开发人员工具以及扩展的代币和访问控制功能。tokens 模块重新设计为统一的 Base 带有 ContractOverrides 模型,以及引入了强大的基于角色的访问控制机制,反映了该系统的深思熟虑和前瞻性的发展。

总的来说,该代码库结构良好,并通过使用表达性强的过程宏而受益于强大的模块化和开发人员人体工程学。但是,某些领域(例如安全实施和授权与初始化等关键模式)仍然是开发人员的责任,并且可以从额外的指导或内置保护中受益。

OpenZeppelin Stellar 开发团队反应迅速,并在整个过程中提供了详细的背景信息。审计团队感谢有机会审查此版本并为 Stellar 生态系统的持续安全性和可靠性做出贡献。

与专家交谈

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

0 条评论

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