OpenZeppelin 对 Stellar Contracts Library v0.3.0-rc.3 版本进行了安全审计,发现了包括严重、高、中、低风险在内的37个问题,该版本引入了架构增强、新功能和扩展的开发者实用程序,特别是通证模块的重构和基于角色的访问控制机制。审计报告强调了潜在的安全风险,并提出了改进建议。
live_until_ledger
超过临时条目的最大 TTL,transfer_role
会发生 panichas_role
宏中支持多种角色transfer_role
仅 TTL 扩展策略可能会超过预期的到期时间类型 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),该版本引入了重大的架构增强、强大的新功能以及扩展的开发者实用工具集。与之前的版本相比,此版本侧重于通过更具凝聚力和可扩展性的架构来改善开发者体验,同时添加了备受期待的、可用于生产环境的功能。
在 RC3 中,最显著的变化之一是对可替代和不可替代Token(NFT)模块进行的深度架构重构。以前的模型依赖于组合多个扩展 trait,现已被更统一和简化的模型所取代。
这种新架构以 Base
合约类型和 ContractOverrides
trait 为中心。核心逻辑(如铸造和元数据处理)现在直接集成到 Base
实现中。这通过减少样板代码来简化标准Token的开发。对于更高级的用例,ContractOverrides
系统允许扩展(如新的 allowlist
和 blocklist
模块)干净且安全地覆盖默认Token行为,例如 transfer
和 approve
,而无需开发者重写整个Token接口。
SAC 管理模块允许用户与 SAC 集成。管理模块支持两种集成方法:通用方法和包装器方法。采用通用方法,sac-admin-generic
模块利用 __check_auth
函数来处理身份验证和授权,从而可以注入自定义授权逻辑。相反,sac-admin-wrapper
模块使管理模块充当中间件,为每个管理函数定义特定的入口点,并将这些调用转发到相应的 SAC 函数。这允许在委托之前应用自定义逻辑,从而实现模块化和直接的设计。
此版本引入了一个全面而灵活的框架,用于管理链上权限,该框架由两个不同的模块组成:
ownable
: 一个用于实现简单的单所有者访问控制的模块。它为需要所有者通过 #[only_owner]
宏授权的函数提供了一个简单的模式。access-control
: 一个功能齐全的基于角色的访问控制(RBAC)系统,用于需要更精细权限的合约。它支持创建自定义角色(例如 minter
、manager
等)、将角色分配给多个帐户,并通过将某些角色指定为其他角色的管理员来建立管理层次结构。这两个模块都具有安全的两步转移机制,用于主要的 owner
或 admin
角色。这种设计选择通过要求提议的新管理员在转移完成之前明确接受该角色来防止意外失去控制。
此版本的一个主要主题是开发者体验的显著改善,这主要得益于强大的过程宏的引入和改进。这些宏旨在减少样板代码,强制执行一致的安全模式,并自动化常见的实现任务,从而使该库更易于访问和更强大。
授权宏: 一套新的属性宏提供了一种干净且声明性的方式来实现访问控制:
#[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实现编写的代码量,并且已扩展为支持新的 AccessControl
和 Ownable
trait 。
可升级合约宏:现有的 #[derive(Upgradeable)]
和 #[derive(UpgradeableMigratable)]
宏已使用一项关键的新功能进行了更新:它们现在会在编译期间自动读取 CARGO_PKG_VERSION
环境变量,并将此版本字符串作为 binver
元数据嵌入到合约的 Wasm 中。这将创建一个不可变的、链上的合约版本记录,为链下工具和升级管理提供关键信息。
NFT 模块中添加了一个新的 royalties
扩展,提供了一个标准化的链上版税支付机制,该机制符合 ERC-2981 标准。这允许创建者和开发者以编程方式直接从他们的 NFT 合约中强制执行版税经济。
该扩展支持:
为了支持更高级的用例(如空投和链下投票),RC3 引入了一组通用的密码学实用工具:
crypto
包提供了通用的哈希原语(Sha256
、Keccak256
等)和一个用于 Merkle 证明的通用验证器。merkle-distributor
模块利用这些原语来为构建通过 Merkle 证明在链上验证声明的应用程序提供一个可重用的模板。这些实用工具在新的示例合约中展示,包括 fungible-merkle-airdrop
和 merkle-voting
,这些合约为开发者提供了实现这些模式的实用模板。
除了上面讨论的主要功能外,此版本还包括:
allowlist
和 blocklist
扩展,使合约管理员可以精细地控制允许哪些帐户与Token进行交互。access-control
框架来管理 SAC 的特权函数。使用此库构建的任何合约的安全性都取决于库本身的正确性和关于其环境和使用的一组信任假设。
范围外的依赖项: 本次审计假设底层 Soroban 环境、目标 WASM 编译器、外部 crate 和 Soroban SDK 的安全性和正确功能。这些组件构成了合约运行的可信基础,但不属于本次审查的直接范围。
开发者责任: Stellar Contracts Library 提供了一组强大但无偏见的原始类型。它不强制执行特定的安全模型,而是为开发者提供构建自己的安全模型的工具。因此,智能合约的最终安全性关键取决于开发者对其的实现。关键责任包括:
Base::mint
)在设计时故意没有内置访问控制。因此,开发者必须将对这些函数的调用包装在强制执行适当授权的函数中,例如,使用提供的 #[only_owner]
或 #[has_role(...)]
宏。否则将导致特权函数可公开调用。owner
或 admin
必须在构造函数中安全设置。未正确配置的合约可能没有管理员,从而使其无法管理。require_auth()
检查会自动通过。因此,假设如果一个合约地址被分配了一个特权角色,则调用合约中会执行适当的授权检查。链下组件: 依赖于链下过程的功能(如 merkle-distributor
)取决于在区块链外部安全且正确地生成数据。虽然本次审计涵盖了链上证明验证逻辑,但它假设提供给合约的 Merkle 根已正确生成并受到信任。
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 #290 的 commit ad09aac 和 pull request #329 的 commit 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 #318 的 commit b303e42 中解决。
Spender
的检查blocklist
扩展的 transfer_from
函数不验证 spender
是否在黑名单中。因此,黑名单中的地址仍然可以代表非黑名单用户执行 transfer_from
操作,前提是它已通过 approve
预先批准。这与 BlockList
trait 相矛盾,该 trait 指定 被阻止的帐户无法转移Token。
这可能会导致混淆,即 from
帐户可能认为黑名单中的 spender 不允许与智能合约交互,尽管该 spender 仍然可以从预先批准的 allowance 中转移Token,从而破坏了黑名单机制的完整性。一个实际的漏洞利用场景可能涉及一个已获得用户批准的受损合约。即使在被列入黑名单之后,该合约仍将能够通过 transfer_from
耗尽用户资金,从而绕过黑名单系统的预期保护。
在 transfer_from
函数中,考虑添加一项检查,以确保 from
和 spender
地址未被列入黑名单。
更新: 已部分解决。该合约仍然允许黑名单中的 spender
与预先批准的Token进行交互。但是,已添加文档以警告 pull request #307 的 commit 14813be 中的此行为。OpenZeppelin Stellar 开发团队表示: 这是遵循以太坊等其他生态系统的惯例所做的有意选择。
access-control
trait 没有提供管理员放弃其角色的方法。这是有问题的,因为 Admin
密钥存储在实例存储中,并且即使不再使用,在 TTL 扩展期间仍将继续被考虑在内,从而增加了操作成本。此外,如果没有放弃机制,就无法有意地使合约永久性地没有管理员,这通常是出于去中心化或升级目的而希望的。
考虑添加一个 renounce_admin
函数,允许当前管理员从角色中删除自己。
更新: 已在 pull request #316 的 commit 0b8dbbd 中解决。
ensure_if_admin_or_admin_role
会发生 panic,并忽略角色管理员ensure_if_admin_or_admin_role
函数 旨在授权作为合约管理员或特定角色管理员的调用者。但是,当前的实现首先检查合约管理员并调用 get_admin
,如果 Admin
密钥不存在于存储中,则会发生 AdminNotSet
错误。这会阻止该函数继续检查调用者是否是角色管理员,从而有效地禁用对没有合约范围管理员的合约中敏感的基于角色的函数(如 grant_role
和 revoke_role
)的访问。
AccessControl
trait 声明 一个合约必须 setting 管理员才能够使用该 trait 的功能。但是,鉴于这是一个库,它应该提供足够的灵活性,以允许在没有管理员的合约中设置角色。想象一个场景,其中管理员分配所需的角色和角色管理员,然后放弃其权力。在这种情况下,相应的角色管理员应该能够控制对其各自角色的访问。
考虑处理 get_admin
在 ensure_if_admin_or_admin_role
函数中返回的 AdminNotSet
错误,并确保及时扩展 Admin
密钥的 TTL。
更新: 已在 pull request #292 的 commit 132ecab 中解决。
在整个代码库中,实例存储的 TTL 没有在任何地方扩展,除了 ownable
trait 的 get_owner
函数。这在 TTL 扩展的处理方式中产生了一种不一致的模式。与其他存储类型不同,扩展实例存储的 TTL 会影响整个合约存储,并且成本可能很高。同时,未在需要时扩展 TTL 可能会导致关键功能丢失,并可能在 TTL 过期后使整个智能合约无法访问。因此,重要的是要明确 TTL 扩展应该在库中处理还是由集成商显式处理。
考虑在整个代码库中扩展实例 TTL,或删除 get_owner
函数中的扩展,并明确记录 TTL 扩展是集成商的责任。
更新: 已在 pull request #293 的 commit 4775033 和 pull request #329 的 commit 68397ff 中解决。
AllowList
和 BlockList
扩展上缺少 FungibleBurnable
实现FungibleBurnable
trait 提供了两个关键函数:burn
和 burn_from
。这些是 SEP-0041 Token接口的一部分,应该可用于任何可替代Token扩展。目前,这些函数已在 Base
合约类型上实现。但是,当尝试将 FungibleBurnable
接口与其他扩展(如 AllowList
或 BlockList
)一起使用时,由于缺少 trait 在这些特定类型上的实现,因此找不到预期的方法。这会阻止用户使用 default_impl macro
生成默认实现,其中 contractType
关联类型指定为 Base
类型以外的其他类型。
考虑将 burn
和 burn_from
函数添加到 AllowList
和 BlockList
实现。
更新:已在pull request #294的commit be401b0中解决。
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 #296的commit 4a27a5e中解决。
代码库中的几个过程宏接受并静默忽略通过 TokenStream
传递的参数,这可能会导致混淆和误用。例如,像 #[only_admin(caller)]
这样的宏在语法上接受 caller
参数,但实现完全忽略它。
具体来说,以下宏会忽略它们的参数输入:
#[only_admin]
— 忽略 access-control-macros
中的 _attrs: TokenStream
#[only_owner]
— 忽略 ownable-macro
中的 _attrs: TokenStream
#[default_impl]
— 忽略 default-impl-macro
中的 _attr: TokenStream
通过接受然后静默地丢弃它们的参数,这些宏给人一种错误的自定义印象:用户可能认为他们正在配置行为(无论是访问检查还是默认实现),而实际上,什么也没发生。外观和操作之间的这种不匹配不仅混淆了代码的意图并使审查复杂化,而且还为不良行为者提供了一个简单的途径来插入欺骗性的注释,这些注释掩盖了未经授权的逻辑或意外的默认值。
为了提高清晰度和开发者体验,请考虑在上述宏中显式地拒绝未使用的参数。这可以通过检查 attr.is_empty()
并使用如下消息触发编译时 panic 来完成:
assert!(attr.is_empty(), "This macro does not accept any arguments");
这将有助于防止误导性使用并提高基于宏的抽象的可靠性和透明度。
更新:已在pull request #295的commit 966775e和pull request #327的commit 110f6ce中解决。
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 #322的commit 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 #311的commit 2bd717a和pull request #330的commit 41060f2中解决。
live_until_ledger
超过临时条目的最大 TTL,则 transfer_role
会 panictransfer_role
函数允许设置一个 live_until_ledger
值来指定临时条目的过期时间。但是,如果提供的 live_until_ledger
隐含的 TTL 超过了临时条目的最大限制,该函数将会 panic。这种行为与持久条目的处理方式不同,在持久条目中,TTLs 被限制为最大值,而不是导致 panic。这可能会导致意外的失败,特别是因为调用者可能合理地期望该函数能够优雅地处理这种情况。在这种情况下,panic 对用户不友好。
考虑在使用 live_until_ledger
输入之前对其进行验证,或者将 TTL 限制为临时条目的最大允许值,从而匹配持久条目的行为。此外,应清楚地记录此 edge case,以防止误用并确保可预测的合约行为。
更新:已在pull request #298的commit ccad351中解决。
set_admin
和 set_owner
函数被设计为仅在智能合约的生命周期内调用一次。这些角色的任何进一步设置都应分别通过 transfer_admin_role
和 transfer_ownership
函数完成。但是,没有验证可以防止 setter 函数被多次调用。
考虑添加一个检查,以使用 e.storage().instance().has(&key)
验证相应的 key 是否已设置,如果存在则使用适当的错误(例如,AdminAlreadySet
或 OwnerAlreadySet
)回退并出现 panic。
更新:已在pull request #299的commit 16bdda0中解决。
openzeppelin-contracts
的开发者感到困惑库中的几个合约使用在 EVM 生态系统中被广泛认可的名称,例如 Ownable
和 AccessControl
,但实现了通常预期的逻辑的变体。例如,名为 Ownable
的合约实现了两步所有权转移模式,该模式更接近 OpenZeppelin EVM 合约库中的 Ownable2Step
。类似地,AccessControl
的实现偏离了标准的 EVM 实现。
鉴于该库正在 OpenZeppelin 品牌下为 Stellar 生态系统开发,从 EVM 环境过渡的用户可能会基于熟悉的合约名称假设相同的语义和使用模式。如果开发者依赖于这些合约中不存在的隐式行为,这可能会导致关于功能的不正确假设、不正确的集成,甚至安全漏洞。
考虑重命名这些合约以更准确地反映它们的行为。或者,考虑提供清晰的文档免责声明和命名说明,以帮助开发者正确理解这些区别。
更新:已在pull request #328的commit e47d949和pull request #330的commit 41060f2中解决。
set_root
和 set_claimed
函数都可以执行敏感的状态更新,而无需任何授权。从示例用法来看,set_root
应该只在构造函数中使用,而 set_claimed
可以是一个私有函数。
对于不安全的函数,考虑添加文档来演示它们的安全用法,并警告它们缺少授权。
更新:已在pull request #322的commit 50cd251中解决。
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 #321的commit 79af2d9中解决。
has_role
宏中支持多个角色当前的 has_role
宏仅验证单个角色,如果调用者缺少该角色,则会回退,迫使开发者在多个角色授予相同权限的情况下复制逻辑。这增加了样板代码并模糊了预期的访问策略。允许 has_role
接受多个角色将允许指定一个角色数组,并在调用者拥有其中任何一个角色时立即授予访问权限。此更改提高了清晰度并减少了重复代码。
考虑扩展 has_role
宏的签名,使其接受角色列表,迭代每个角色,并且如果任何检查通过则允许执行,同时保持对单角色检查的向后兼容性。
更新:已在pull request #325的commit e7251be中解决。
在 access-control
模块中,可以以循环方式分配 RoleAdmin。例如,可以将 MINT_ADMIN
分配为 MINT_ROLE
的管理员,同时让 MINT_ROLE
成为 MINT_ADMIN
的管理员。但是,当发生这种情况时,可能会产生意想不到的后果。例如,当一个角色是另一个角色的管理员时,可能会创建一个角色撤销另一个角色的竞争条件。
由于 set_role_admin_no_auth 是一个受限制的函数,如果无法在代码中阻止上述行为,请考虑警告上述行为。
更新:已在pull request #312的commit e0a8118中解决。
在整个代码库中,发现了多个代码优化的机会:
block_user
、unblock_user
、allow_user
和 disallow_user
等函数总是执行存储写入并发出事件。false
,allowed
和 blocked
函数也会延长 TTL。disallow_user
和 unblock_user
显式地存储 false
而不是删除条目,导致不必要的存储费用和 TTL 跟踪开销。为了避免产生不必要的成本,请考虑添加 if
检查以仅在新状态与当前状态不同时才执行存储写入(并发出事件)。此外,当撤销访问权限或解除阻止用户时,请考虑使用以下代码删除条目,而不是写入 false
。
e.storage().persistent().remove(&key);
更新:已在pull request #303的commit bf51ae5中解决。
代码重复会导致冗余操作并浪费计算成本。
在 grant_role_no_auth
函数中,AccessControlStorageKey::HasRole(account, role)
密钥首先在 add_to_role_enumeration
函数中设置,然后在 grant_role_no_auth
函数中再次设置。
考虑删除任何重复代码的实例。
更新:已在pull request #304的commit 7158c50中解决。
AccessControlStorageKeys
可以使用 set_role_admin
函数中的相同角色密钥设置和重置 AccessControlStorageKeys.RoleAdmin(RoleSymbol)
。但是,对于不再使用的角色密钥,无法将其删除。每次调用 get_role_admin
函数时,都可以延长其 TTL,从而增加调用者的成本。
类似地,当 AccessControlStorageKeys.RoleAccountsCount(RoleSymbol)
为零时(例如,当角色符号不再使用时),无法删除 ledger 密钥。因此,当调用 get_role_member_count
函数时,可以延长 TTL,从而增加调用者的成本。
考虑允许管理员删除不再使用的角色的未使用密钥。
_更新:已在pull request #306的commit d9d3ab5中解决。该 pull request 引入了 remove_role_admin_no_auth
和 remove_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
是一个上限,而不是保证的到期时间live_for
短于默认的最小 TTL,则该entry将比 live_until_ledger
存活得更久在文档中包含这些说明 将有助于确保开发者理解实际到期可能超过指定的 ledger。
更新:已在pull request #323的commit 72053cb中解决。
在整个代码库中,发现了多个具有误导性和/或不准确性的文档实例:
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 #321 的 commit 79af2d9 中解决。
代码库在处理与缺失密钥和授权失败相关的 panic 时存在不一致性。
例如,get_admin
函数从实例存储中检索 Admin
密钥,如果未找到该密钥,则 直接 panic。此函数在 enforce_admin_auth
中使用,如果在授权失败时执行的是隐式 panic,而没有显示有意义或结构化的错误。相比之下,如果缺少 Owner
密钥,get_owner
函数返回 None
。当 在 enforce_owner_auth
中使用时,它会检查授权并 panic 并显示 NotAuthorized
错误。但是,如果实际问题是所有者密钥不存在,则此错误可能会产生误导。
考虑通过使用带有清晰且不同的错误消息的 panic_with_error!
来标准化缺失密钥和失败授权的处理。
更新:已在 pull request #326 的 commit 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.0
和 ed25519-dalek 1.0.1
。
虽然上述依赖项仅在示例中使用过,但请考虑将其升级到最新的、非易受攻击版本。或者,考虑重构或删除受影响的示例代码,以消除对易受攻击 crate 的依赖。
更新:已在 pull request #319 的 commit a619477 中解决。
Stellar 文档 明确警告说,在更新 SAC 管理员地址时,不会在更改期间验证新的管理员地址。因此,可能会无意中分配无效或不正确的管理员地址,这可能会不可逆转地锁定 SAC 的管理控制。
虽然这对于任何管理智能合约都是如此,但请考虑将此警告添加到 SAC 管理员相关的库文档中,以反映风险并与 Stellar 文档保持一致。
更新:已在 pull request #320 的 commit 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 #308 的 commit dab8fc5 中解决。
某些错误的当前实现将多个语义上不同的失败场景合并为一个通用错误,从而大大降低了开发人员的清晰度和故障排除能力。尽管发生在不同的上下文中,但错误消息保持不变,没有提供有关失败根本原因的任何信息:
AccessControlError::AccountNotFound
错误用于指示各种不相关的问题。在 get_role_member
函数中,当提供的索引超出角色的成员列表的范围时,会触发错误,更合适的错误是 IndexOutOfBounds
之类的描述性错误。在 revoke_role
和 renounce_role
函数中,当目标帐户没有指定的角色时,会发生错误,最好表示为 RoleNotHeld
。在 remove_from_role_enumeration
函数中,错误可能源于角色没有成员或指定的帐户不是角色的一部分,最好用不同的错误(例如 RoleIsEmpty
和 RoleNotHeld
)来更准确地捕获该错误。
AccessControlError::Unauthorized
错误在 ensure_if_admin_or_admin_role
函数中使用,以指示调用者既不是合约管理员也不是角色管理员,但是,当未设置角色管理员时也会抛出此错误,最好用诸如 RoleAdminNotHeld
之类的错误来更准确地捕获该错误。
当将不存在的角色与 has_role
宏一起使用时,将返回 AccessControlError::Unauthorized
错误消息,该错误消息与帐户没有现有角色时返回的错误消息相同。
考虑重构 AccessControlError
枚举以包含更精细的变体,以反映正确的错误消息。此更改可为开发人员提供即时、与上下文相关的反馈,并提高合约的可维护性。
更新:已在 pull request #309 的 commit 86d038d 中解决。
transfer_role
中的边缘情况导致 NoPendingTransfer
live_for
值 计算 为 live_until_ledger
和 current_ledger
之间的差。如果 live_until_ledger
和 current_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
node
通常指内部节点,而 leaf
用于指树的成员。因此,IndexableNode
可以更准确地重命名为 IndexableLeaf
。同样,node 函数参数可以命名为 leaf
,局部变量 leaf 可以命名为 leafHash
以区别于参数。考虑实施上述建议以提高代码库的清晰度和可维护性。
更新:在 pull request #313 的 commit f0a4dae 和 pull request #322 的 commit 50cd251 中部分解决。OpenZeppelin Stellar 开发团队表示:
我们已接受了 3 项建议中的 2 项。但是,我们不会用
unsafe
替换no_auth
。这是因为在 Rust 中,unsafe
具有不同的含义。这意味着我们为了更大的灵活性和优化而选择退出 Rust 编译器的安全保证。我们的no_auth
函数并非如此。
enforce_admin_auth
和 enforce_owner_auth
函数分别返回 admin
和 owner
值。但是,这些值未在 onlyAdmin
或 onlyOwner
宏的上下文中被使用,从而使宏的使用成本略高。enforce_owner_auth
在 transfer_ownership
函数 中被调用,其中 owner
返回值正在被使用。相反,transfer_admin_role
调用 get_admin
和 admin.require_auth()
而不是调用 enforce_admin_auth
函数。采用一致的方法来使用这些返回值将提高代码的可读性。
为了提高代码库的一致性和可维护性,请考虑在 transfer_admin_role
函数中使用返回值或完全删除它们。
更新:已在 pull request #315 的 commit a954d5d 中解决。
在 renounce_ownership
函数中,临时存储中的 PendingOwner
密钥会 被检查,以确定所有权转移是否正在进行中。如果密钥存在,则 TTL 会被扩展,并且在此之后,该函数会 立即恢复,表明如果所有权转移正在进行中,则不应允许放弃所有权。鉴于该函数在 TTL 扩展后立即 panic,因此扩展会被回滚并且不起作用,从而导致计算成本浪费。
考虑从 renounce_ownership
函数中删除 TTL 扩展。
更新:已在 pull request #314 的 commit a15bf50 中解决。
non-fungible
特性的 royalties
扩展 在设置版税时不会发出任何事件。
考虑在与版税相关的状态更改时发出事件。
更新:已在 pull request #251 的 commit 4267970 中解决。
一些支持的特性没有通过 default_impl
macro 进行默认实现。此类特性包括 FungibleAllowList、FungibleBlockList 和 NonFungibleRoyalties。
考虑包含所有支持的特性的默认实现。
更新:已在 pull request #252 的 commit e7d8120 中解决。
经审计的代码库引入了 Stellar Contracts Library 的 Release Candidate 3 (RC3),该版本带来了重要的架构改进、增强的开发人员工具以及扩展的代币和访问控制功能。tokens
模块重新设计为统一的 Base
带有 ContractOverrides
模型,以及引入了强大的基于角色的访问控制机制,反映了该系统的深思熟虑和前瞻性的发展。
总的来说,该代码库结构良好,并通过使用表达性强的过程宏而受益于强大的模块化和开发人员人体工程学。但是,某些领域(例如安全实施和授权与初始化等关键模式)仍然是开发人员的责任,并且可以从额外的指导或内置保护中受益。
OpenZeppelin Stellar 开发团队反应迅速,并在整个过程中提供了详细的背景信息。审计团队感谢有机会审查此版本并为 Stellar 生态系统的持续安全性和可靠性做出贡献。
- 原文链接: blog.openzeppelin.com/st...
- 登链社区 AI 助手,为大家转译优秀英文文章,如有翻译不通的地方,还请包涵~
如果觉得我的文章对您有用,请随意打赏。你的支持将鼓励我继续创作!