Fireblocks 可升级代币审计 - ERC20F

目录

• 摘要
• 范围
• 系统概述
  • 特权角色
  • 信任假设
• 低严重性
  • 获取整个访问列表可能会回滚
• 备注与附加信息
  • 缺乏安全联系
  • 不必要的变量强制转换
  • 未使用的导入
  • 缺乏 __gap 变量
  • AccessListUpgradeable 中存储间隙大小不正确
  • 使用了令人困惑的错误
  • 缺少输入验证
  • 排版错误和错误注释
  • Gas 优化
  • 缺少恢复 ERC-721 和 ERC-1155 代币的功能
• 结论

摘要

类型：DeFi

时间：从 2023-11-13 到 2023-11-17

语言：Solidity

总问题：11 (7 已解决)

严重性：

• 严重性问题：0 (0 已解决)
• 高严重性问题：0 (0 已解决)
• 中严重性问题：0 (0 已解决)
• 低严重性问题：1 (1 部分解决)

备注与附加信息：10 (7 已解决)

范围

我们审计了 Fireblocks 代码库的提交 e7003dfb。

审计结束后，Fireblocks 将许可证更新为 AGPL-3.0-or-later，在 pull request #1 中的提交 6d8327b。此更新对源代码逻辑的安全性没有影响。

审计范围包括以下合约：

 contracts
├── ERC20F.sol
└── library
    ├── AccessRegistry
    │   ├── AccessListUpgradeable.sol
    │   ├── AccessRegistrySubscriptionUpgradeable.sol
    │   ├── AllowList.sol
    │   ├── DenyList.sol
    │   └── interface
    │       └── IAccessRegistry.sol
    ├── Errors
    │   ├── LibErrors.sol
    │   └── interface
    │       └── IERC20Errors.sol
    └── Utils
        ├── ContractUriUpgradeable.sol
        ├── MarketplaceOperatorUpgradeable.sol
        ├── PauseUpgradeable.sol
        ├── RoleAccessUpgradeable.sol
        ├── SalvageUpgradeable.sol
        └── TokenUriUpgradeable.sol

更新：该代码库已迁移到新的代码库 https://github.com/fireblocks/fireblocks-smart-contracts，经过审计，直到提交 a8942e9 在 PR #3。此 PR 后来在提交 108a92f 中合并到 main 分支。

系统概述

Fireblocks Upgradeable Tokens 项目实现了可升级的代币合约，称为 ERC20F，它符合 ERC-20 代币标准，并集成了额外的功能来管理访问控制。该代币便于为代币转账配置访问列表，灵活地实现白名单，只有列表中的地址可以使用代币，或黑名单，限制列表中的地址与合约交互。

特权角色

该协议实施了多个特权角色，这些角色与 ERC20F 合约相关联。

持有 DEFAULT_ADMIN_ROLE 的用户可以：

• 授予其他地址角色。

持有 UPGRADER_ROLE 的用户可以：

• 将实施合约升级到新版本。

持有 PAUSER_ROLE 的用户可以：

• 暂停和恢复合约。

持有 CONTRACT_ADMIN_ROLE 的用户可以：

• 更改所使用的访问列表地址。
• 更新合约 URI。

持有 MINTER_ROLE 的用户可以：

• 铸造代币。

持有 BURNER_ROLE 的用户可以：

• 销毁代币。

持有 RECOVERY_ROLE 的用户可以：

• 从缺乏访问权限的账户中恢复代币。

持有 SALVAGE_ROLE 的用户可以：

• 从代币合约本身恢复发送进去的 ETH 和 ERC-20 代币。

信任假设

• DEFAULT_ROLE_ADMIN 角色的持有人预计是非恶意的，并以项目的最佳利益行事。

低严重性

获取整个访问列表可能会回滚

AccessListUpgradeable 合约包含一个 getAccessList 函数，该函数用于从访问列表中检索所有值。此函数利用 EnumerableSetUpgradeable.AddressSet 的 values 函数，将整个存储复制到内存中。当访问列表过大时，执行函数可能会因为受到块 gas 限制的影响而导致调用回滚。

考虑实现一个附加的函数，当值集过大时可以使用。此函数应仅返回整个集合中的一个子集或切片。

更新： 已确认，部分解决在提交 130dbb5。Fireblocks 团队表示：

为了解决这个问题，我们通过将额外的文档加入到该函数中来解决。此决定是因为 getAccessList() 的目的旨在成为一个支持功能，旨在为较小项目提取紧凑访问列表。在带有更大访问列表的大型项目中，建议设计自定义索引逻辑，以有效检索所有地址。

备注与附加信息

缺乏安全联系

在智能合约中提供一个特定的安全联系（例如电子邮件地址或 ENS 名称），可以大大简化人与人之间的沟通，以便在他们发现代码中的漏洞时联系代码所有者。这种做法是有益的，因为它允许代码所有者控制漏洞披露的沟通渠道，从而消除因不知如何报告而导致的误沟通或未提交报告的风险。此外，如果合约包含第三方库，并且出现漏洞，这使得那些库的维护者与相关人员联系关于问题并提供缓解建议变得更容易。

考虑在合约定义上方添加一个包含安全联系的 NatSpec 注释。推荐使用 @custom:security-contact 约定，因为它已被 OpenZeppelin Wizard 和 ethereum-lists 采用。

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

由于这些合约是作为模板，供其他公司和实体大规模部署。因此不能有单一的安全联系。因此，当公司部署其中一个合约时，他们可以通过合约 URI 独立提供自己的安全联系。

不必要的变量强制转换

在 AccessRegistrySubscriptionUpgradeable 合约中，_accessRegistry 变量在 _accessRegistryUpdate 函数中被不必要地强制转换。

为了改善代码库的整体清晰度、意图和可读性，考虑删除任何不必要的强制转换。

更新： 已确认，在提交 69c5add 中解决。

未使用的导入

在整个 代码库 中，有多个未使用的导入可以删除：

• import {LibErrors} from "../Errors/LibErrors.sol"; 在 ContractUriUpgradeable.sol 中导入未使用的别名 LibErrors
• import {LibErrors} from "../Errors/LibErrors.sol"; 在 MarketplaceOperatorUpgradeable.sol 中导入未使用的别名 LibErrors
• import {LibErrors} from "../Errors/LibErrors.sol"; 在 PauseUpgradeable.sol 中导入未使用的别名 LibErrors
• import {LibErrors} from "../Errors/LibErrors.sol"; 在 TokenUriUpgradeable.sol 中导入未使用的别名 LibErrors

考虑去掉未使用的导入，以提高代码库的整体清晰度和可读性。

更新： 已确认，在提交 4243399 中解决。

缺乏 __gap 变量

在整个 代码库 中，有多个可升级合约没有 __gap 变量。例如：

• AllowList 合约
• DenyList 合约
• ERC20F 合约

请注意，当前的实现并不需要 __gap 存储变量，因为列出的合约不需要被继承。然而，仍然建议包括它，因为它意味着合约实现可能会被升级，未来可能会引入额外的存储变量。

考虑添加 __gap 存储变量，以避免在可升级合约中出现未来存储冲突。

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

_缺乏 __gap 变量的合约在我们的用例中并不需要，因为升级是通过合约继承进行的。在这个过程中，初始实现 V1 被 V2 继承，其中包含新的逻辑。这使得添加存储槽时不会与现有槽发生冲突。_

AccessListUpgradeable 中存储间隙大小不正确

AccessListUpgradeable 合约使用了一个大小为 49 的 __gap 存储变量。它假定 _accessList 存储变量占用一个存储槽，而实际上它占用两个存储槽。_accessList 的类型是 EnumerableSetUpgradeable.AddressSet，这是一个结构，使用了 2 个存储槽。

考虑将 AccessListUpgradeable 合约中的 __gap 存储变量的大小从 49 改为 48。

更新： 已确认，在提交 519e415 中解决。

使用了令人困惑的错误

ERC20F 合约具有一个 recoverTokens 函数，旨在从缺乏访问权限的账户中恢复代币。问题出现在账户已经可以访问代币时，导致交易回滚并出现潜在的混淆错误，例如 ERC20InvalidReceiver。在这种情况下，由于该账户不是接收者，错误不应暗示接收者相关问题。

考虑添加一个新错误，指示该账户可以访问代币。

更新： 已确认，在提交 6b3f311 中解决。

缺少输入验证

ERC20F 合约的 burn 函数缺少检查，以确保数量不等于 0。

考虑实现建议的验证，以防止意外行为，可能导致对协议的潜在攻击。

更新： 已确认，在提交 6953e70 中解决。

排版错误和错误注释

考虑解决以下排版错误和错误注释：

• AccessRegistrySubscriptionUpgradeable 合约实现了 __AccessRegsitrySubscription_init 函数，但其名称中有拼写错误，应该为 __AccessRegistrySubscription_init。
• 描述 AccessListUpgradeable 中实现的 hasAccess 函数的 NatSpec 注释使用了参数名 claim 而不是 data。

更新： 已确认，在提交 7a5e68e 中解决。

Gas 优化

在代码库中，有多个潜在的 Gas 优化实例：

• ERC20F 合约中的 _requireHasAccess 函数冗余地检查 isSender 值两次。更有效的方式是首先通过 accessRegistry 合约确认账户是否有访问权限。如果没有访问权限，则根据 isSender 值，回滚并返回无效发送者或接收者错误。
• 代码库遵循通常在更新状态变量后的访问检查模式。例如，在 RoleAccessUpgradeable 合约中，revokeRole 函数在更新角色后调用了 _authorizeRoleAccess 函数。为了优先考虑快速失败和节省 Gas，考虑在更新状态变量之前先执行访问检查。

为了改善 Gas 消耗、可读性和代码质量，考虑尽可能进行重构。

更新： 已确认，在提交 f84408b 中解决。

缺少恢复 ERC-721 和 ERC-1155 代币的功能

SalvageUpgradeable 抽象合约，继承自 ERC20F 合约，实施了 恢复 ETH 和 ERC-20 代币 的逻辑。这对于当代币意外发送到代币合约本身而不是有效接收者时是有用的。然而，恢复逻辑仅适用于恢复 ETH 和 ERC-20 代币，并不适用于 ERC-721 或 ERC-1155 代币。因此，虽然不小心发送到任何三个代币合约中的 ERC-20 代币可以得到恢复，但 ERC-721 或 ERC-1155 代币则无法恢复。

考虑添加恢复 ERC-721 和 ERC-1155 代币的功能，这些代币意外发送到 ERC20F 合约。

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

目前对恢复 ERC-721 或 ERC-1155 代币没有观察到客户需求。虽然这个功能可能在之后通过升级纳入，但我们希望谨慎处理，以免向用户施加额外的字节码和 Gas 费用，因为这个功能目前可能无法使用。

结论

Fireblocks Upgradeable Tokens 项目引入了可升级的代币合约，符合 ERC-20 标准，并添加了用于访问控制的功能。该代币允许为转账创建访问列表，使得能够实现白名单，限制使用指定地址，或黑名单，阻止与列表中地址的交互。

发现了一个低严重性问题。此外，提出了多项建议以提高代码库的质量和文档。Fireblocks 团队响应及时，回答了我们对代码库的所有问题。

• 原文链接： blog.openzeppelin.com/fi...
• 登链社区 AI 助手，为大家转译优秀英文文章，如有翻译不通的地方，还请包涵～