本文是对 Across 协议代码的审计报告,重点关注了引入 LayerZero OFT 桥接机制的更改,并评估了其安全性、集成性和潜在风险。报告内容包括高、中、低风险的问题,以及关于代码质量、测试覆盖率和文档的改进建议。同时,报告也强调了协议中存在的信任假设和未来集成时需要注意的关键点。
TypeCross-ChainTimelineFrom 2025-05-19To 2025-05-23LanguagesSolidityTotal Issues32 (11 resolved, 4 partially resolved)Critical Severity Issues0 (0 resolved)High Severity Issues1 (0 resolved)Medium Severity Issues7 (4 resolved, 1 partially resolved)Low Severity Issues13 (1 resolved, 3 partially resolved)Notes & Additional Information11 (6 resolved)
OpenZeppelin 对 across-protocol/contracts 仓库在提交 c5d75410 处,相对于基础提交 c88ac8ad 进行了差异审计。 特别是,此差异 中高亮显示的变化是本次审计的主要内容。
此外,还审计了以下两个 pull request:
以下文件在审计范围内:
contracts
├── AdapterStore.sol
├── AlephZero_SpokePool.sol
├── Arbitrum_SpokePool.sol
├── Ethereum_SpokePool.sol
├── Linea_SpokePool.sol
├── Ovm_SpokePool.sol
├── PolygonZkEVM_SpokePool.sol
├── Polygon_SpokePool.sol
├── Scroll_SpokePool.sol
├── SpokePool.sol
├── Succinct_SpokePool.sol
├── Universal_SpokePool.sol
├── ZkSync_SpokePool.sol
├── chain-adapters
│ ├── Arbitrum_Adapter.sol
│ └── Universal_Adapter.sol
├── libraries
│ ├── OFTTransportAdapter.sol
│ └── OFTTransportAdapterWithStore.sol
└── interfaces
├── SpokePoolInterface.sol
└── V3SpokePoolInterface.sol
Across 是一个跨链桥接协议,围绕以太坊上的中心 HubPool
合约和部署在受支持网络上的每个链的 SpokePool
合约构建。当用户发起跨链转账时,Relayer(filler)会观察到该意图并在目标链上先行提供流动性,从而实现快速结算。一个 Dataworker 网络提交这些填充事件的证明,协议使用这些证明来验证其有效性并计算相应的 Relayer 的补偿。成功验证后,协议不会直接在目标链上补偿 Relayer。相反,Relayer 可以决定它想要在哪里获得补偿。HubPool
合约管理跨链结算,维护全局状态,并执行最终的 Token 补偿和记账。
这些更改 通过将 LayerZero 的消息传递标准集成到 Across 的 Transport Layer 中,引入了对桥接 LayerZero OFT (Omnichain Fungible Token) 资产(如 USDT0)的支持。主要的桥接逻辑位于 OFTTransportAdapter
合约中,该合约在两个方向上都得到使用:
L1->L2 转账:在 L1 端,选择的 Adapters(例如,Arbitrum_Adapter
)继承自 OFTTransportAdapterWithStore
合约,该合约使用全局 AdapterStore
合约来将 Tokens 映射到其对应的 OFT messengers。
L2->L1 转账:在 L2 端,每个 SpokePool
合约(例如,Arbitrum_SpokePool
)通过基础 SpokePool
合约继承 OFTTransportAdapter
合约,并通过 setter/getter 模式维护其自己的本地 Token-messenger 映射。
在这两种流程中,_transferViaOFT
函数确保正确的 LayerZero 手续费报价,应用最大允许手续费 FEE_CAP
,并调用 OFT messenger 的 send
函数以跨链中继 Tokens。这种集成允许 Across 在其自己的 Relayers、fill proofs 和通过基于以太坊的 HubPool
合约进行结算的经济模型中包装 LayerZero 的 OFT 标准以进行跨链转账。
请注意,上面描述的桥接机制不用于发送用户资产。如前所述,这些资产由 Relayers 交付,Relayers 在目标链上先行提供流动性。相反,通过新引入的 _transferViaOFT
函数进行的桥接是协议内部记账的一部分,用于在 SpokePool
合约和 HubPool
合约之间重新平衡流动性,并通过将 Tokens 从 L2 桥接到 L1 HubPool
合约来补偿 Relayers。在 Relayer 在 L1 上填充转账的特殊情况下(例如,对于 L2->L1 用户请求),补偿完全发生在 L1 上,而无需使用上述函数。
Pull request #1031 删除了传统功能,该功能提供了将 outputToken
的地址设置为零地址的选项。这过去表示 output Token 应被解释为目标链上 input Token 的等效 Token。删除此功能的动机是无法确定某些链的等效 Token。以前,等效是通过 Pool 重新平衡路由确定的,该路由过去连接所有受支持的 Tokens。然而,情况不再如此,即并非所有 Tokens 都具有连接的 Pool 重新平衡路由。
Pull request #1032 增加了对 EIP-7702 委托钱包接收 ETH 而不是包装 Tokens(例如,WETH)的支持。后者是常规智能合约的默认 Token 类型。
在审计期间,识别出以下信任假设:
SpokePool
管理员SpokePool
管理员被信任以诚实和负责任的方式行事。具体来说:
SpokePool
管理员被信任以正确地将 messenger 映射到 Token 合约地址,特别是通过 setOftMessenger
函数,该函数允许覆盖 oftMessengers
映射中的值,而无需进行彻底的验证。恶意或诚实但疏忽的管理人员可能将 OFT messenger 映射到错误的 Token 地址,甚至映射到非 Token 地址的风险由该信任假设涵盖。SpokePool
合约的管理员不会通过 setOftMessenger
函数 更改 messenger 的 Token 映射,同时 Token 正在通过该方法传输,并且交易尚未在源头开采。在这种情况下,转账可能会在最佳情况下 Revert 或导致不一致。SpokePool
合约的管理员完全控制中继配置。具体来说,可能 最终得到一种配置,其中 USDC Tokens 通过 OFT 桥中继(而不是通过 CCTP)。此外,即使在配置 OFT 桥以中继 USDC Tokens 之后,如果 CCTP 桥在此期间启用,它们仍然可能 通过 CCTP 桥中继。AdapterStore
管理员存在对 AdapterStore
管理员诚实的信任假设。具体来说,假设他们 将 IOFT messengers 设置为 AdapterStore
合约中的有效 IOFT 地址。
对 LayerZero 后端实现存在隐式信任:
send
方法 的调用返回两个结构体:MessagingReceipt
和 OFTReceipt
。 在这些结构体中,只检查后者,信任该操作在 LayerZero 端是成功的。 特别的,此信任涵盖了这样一种场景,其中 OFT Layer 返回收据,但消息传递仍然在目标端失败,潜在结果是资金损失。quoteSend
方法, 在 OFTTransportAdapter
合约的 _transferViaOFT
函数调用中被调用,被信任会“正确地”行事。 具体来说,信任 LayerZero 不会不合理地增加 quoteSend
返回的 fee
,例如,增加到接近 OFT_FEE_CAP
的值,目的是从重放过程中提取最大经济利润,从而可能通过使其在经济上不可行来停止执行。对于 EIP-7702 委托钱包,做出以下信任假设:
fallback
或 receive
函数,这将阻止它们接收 ETH。 类似地,如果钱包没有实现各自资产的 hooks(例如 ERC-1155 或 ERC-721 中找到的 hooks),则这些将无法接收这些资产,这将导致交易 Revert。以下通用信任假设是审计的一部分:
SpokePool
合约的 _unwrapwrappedNativeTokenTo
函数 使用 OpenZeppelin 合约库中的 isContract
函数来检查地址是否对应于 EOA 帐户。 然而,isContract
函数是该库的旧版本 (4.x) 的一部分,并且已在最新版本 (5.x) 中删除。 鉴于已知的风险,强烈建议不要使用 isContract
,这些风险在库文档中 列出(参见标有 "IMPORTANT" 的部分)。 值得一提的是,这也适用于引入像合约一样运行的 EIP-7702 EOA 钱包。
以下是在未来集成中应牢记的一些重要细节:
_transferViaOFT
函数 对 messenger 的 send
方法进行 外部调用,其中 messenger 映射到给定的 LayerZero 支持的 Token。 原则上,这种外部调用可能会为重入攻击打开大门。 目前,这不是一个问题,因为 _transferViaOFT
函数由 nonReentrant
修饰符保护的函数调用。 但是,在未来的集成中,必须意识到这一事实,并通过在 _transferViaOFT
函数中添加非重入功能来解决它。Ethereum_SpokePool
合约中,__SpokePool_init
函数 的第二个参数(应该是 _crossDomainAdmin
)设置为与第三个参数 ( _withdrawalRecipient
) 相同。文档指出 "crossDomainAdmin
在此合约上未使用"。 但是,可以理解的是,这里存在一个假设:对于有问题的函数,HubPool
合约将始终是提款接收者和管理员。 在未来的集成中,建议记住此假设并验证它是否成立。 否则,该地址可能会获得不应拥有的额外权限。outputToken
Token 中使用零地址来表示与路由器提供的 Token 等效。 应记住这一点,以在未来和过去的集成中保持一致性。OFTTransportAdapter
合约的实现 限制了 LayerZero 的额外选项、可组合消息和命令的使用。 如果某些转账或资产确实需要使用它们,则结果可能不是预期的。AdapterStore
合约允许使用不同的 messenger 类型作为 crossChainMessengers
映射 的键。 但是,当前可以使用 AdapterStore
合约的所有适配器都不允许选择此类类型作为其值 是硬编码的。 这意味着管理员可以设置其类型与 OFT_MESSENGER
类型不匹配的 messengers,但 messengers 将不会被使用。 此外,它们可能仍然没有被注意到,直到它们将来可以执行恶意操作。SpokePool
合约的所有部分,也会允许使用 OFT Transport,即使它们继承了逻辑。 这些合约使用硬编码的值,虽然它们 未直接限制 OFT Transport 功能,但使其无用。 但是,值得一提的是,可以通过从基础合约 ( SpokePool
) 中取出 OFT Transport 逻辑来进一步减少攻击面。Arbitrum_Adapter
和 Arbitrum_SpokePool
合约实现了一个新的 else if
条件语句分支来使用 OFTTransportAdapter
合约中的 _transferViaOFT
函数。 要进入此分支,唯一的要求是 与该 Token 关联的 messenger 不为零。
HubPool
合约端的调用栈是:executeRootBundle -> _sendTokensToChainAndUpdatePooledTokenTrackers -> relayTokens -> _transferViaOFT。 在此调用序列中,如果 messenger 在其内部操作中 发送消息 到最里面的 _transferViaOFT
函数中抛出错误,则 Revert 将一直传播到 _sendTokensToChainAndUpdatePooledTokenTrackers
函数,并且资产 将不会被发送。 不仅如此,由于资产 与包含退款根的消息在同一调用中发送,因此根也将不会发送到相应的 SpokePool
合约。 由于桥接机制是按顺序检查的,因此 Arbitrum Gateway 在 OFT Transport 方法之后出现,将无法用作替代品,这意味着在 messenger 解决冲突或管理员角色从 oftMessengers
映射中删除它之前,将无法转移这些资金,这可能会导致僵局,直到它得到解决。
为了防止在 OFT messenger 当前无法正常工作时,需要将资产移入或移出 Arbitrum 的情况(例如,没有足够的资产),请考虑在所有受影响的合约中替换这些模式,以允许绕过失败的 messenger,以便可以使用标准的替代方法。 或者,考虑使用 try-catch
机制来防止 Revert 的传播,这也可能影响根哈希的桥接。
更新: 已确认,未解决。 团队表示:
这确实是一个重要的情况,但是,try - catching 不会缓解这种情况。
我们设想的情况是 OFT transfer 对于某些 OFT Token 停止工作(messenger 冻结路由),例如 USDT。 正如在 Slack 中讨论的那样,无法通过标准桥以有意义的方式将 USDT 桥接到 L2。 如果该路由的 OFT messenger 被关闭/冻结,那么确实我们的系统无法将 USDT 重新平衡到该目标链,因为 OFT 应该被认为是 USDT 唯一可用的桥接选项。
但是,如果发生这种情况,工程团队必须立即了解。 我们不能只是默默地尝试捕获,因为系统将无法在没有它所需的重新平衡的情况下继续正常运行(例如,我们尝试用 Spoke 余额中没有的 USDT 在 Arbitrum 上偿还 relays -> 这是行不通的)。 我们设想的缓解方案如下:
_1. 我们部署了一个新的
ArbitrumAdapter
,其中注释掉了_transferViaOft
行,允许捆绑包执行。_2. 此时,任何 Arbitrum 上留下的 USDT 中继器退款都会失败并大量通知我们。 我们通过修改executor来抑制这种噪音来处理这个问题(可能)。
_3. 一旦这个路径的 OFT 恢复冻结,我们就可以将 relaySpokePoolAdminFunction`与 Arbitrum_SendTokensAdapter.sol(更改它的代码到 OFT 逻辑),以回填 L2 上缺少的 USDT Token(在 1 期间未正确执行)。_
否则,如果 OFT 路由永远不会解冻,我们将无法偿还 Arbitrum 上的 relays 。 总而言之,除了通过 OFT 适配器发送资金之外,任何其他操作都需要人工干预。 (即没有 try-catch 并放弃发送资金)。
OFTTransportAdapter
中发送金额的验证不足在 OFTTransportAdapter
合约中,transferViaOFT
函数 获取包含两个元素的 OFTReceipt
输出:源头发送的金额和目标收到的金额。 当前的实现仅验证目标收到的金额,确保它与用户指定的输入金额匹配。 但是,这种方法忽略了对发送金额的验证,从而可能增加攻击面。
由于缺少对源头发送的值的验证,因此可能会实现这样一种情况:messenger 可能会在源头获取更多资产,在目标处存入正确的较少量,并通过检查。 LayerZero 关于 _debit
函数的文档 指出 “在 NON-default OFT 中,amountSentLD 可能是 100,费用为 10%,amountReceivedLD 金额为 90,因此 amountSentLD 可能与 amountReceivedLD 不同。” 当前的实现尝试通过与 Token 关联的 forceApprove
调用 方法来缓解这种情况。 尽管如此,如果它的实现允许比发送的金额更灵活或更大的值,结果将取决于 messenger 不会拿超过需要的假设。
考虑对源头发送的值施加限制,以减少攻击面并防止上述情况的发生。 此外,考虑验证通过 OFT messenger 使用的 Token 在使用 approval 功能时是否不会表现出行为偏差或边缘情况。
更新:在 pull request #1027 中已解决。
在 SpokePool
合约中,_setOftMessenger 函数允许为特定 Token 添加或更新 OFT messenger 地址。 但是,如果当前的 messenger 受到损害或需要删除,则没有允许管理员从可用 messengers 中删除它的功能(无需用新的替换它)。
此外,无法使用 _setOftMessenger
函数将其设置为零,因为对其 Token 进行了 验证,如果 messenger 设置为零地址或没有实现 token
方法的地址或与传递的地址不匹配的地址,则会 Revert。 由于临时将(受损的)messenger 地址设置为实现 token
方法的虚拟合约也可能很危险,因为它可能不会停止整个 OFT 流程,因此管理员可能无法足够快地做出反应并停止使用它。
考虑实现一种允许管理员从存储中删除 messenger 的方法。
_更新: 在 pull request #1034 中已解决。 团队表示:即使 OFTTransportAdapter
合约指示它将以原生代币支付费用,但并没有验证 ZRO 代币的费用值是否为零,这意味着它将再次传递给消息传递者。这意味着如果消息传递者的实现同时输出两个报价,它可能无法识别将使用哪种资产支付,如果尝试使用 ZRO 支付,则会导致回滚。正如 LayerZero 的一个例子 中所见,在以原生代币支付的情况下,lzTokenFee
参数设置为零。
为了防止意外结果和回滚,特别是如果消息传递者的行为发生偏差,请考虑在报价时断言返回的 lzTokenFee
为零。此外,请考虑向 模拟合约 添加更多场景,以验证与协议的正确集成。
更新:已在 pull request #1029 中解决。 团队声明:
添加了对
lzTokenFee
的零检查,并添加了一些费用负面场景测试(部分解决了 M-04)。
在整个代码库中,特别是在添加的更改中,发现了多个测试覆盖率不足的实例:
测试不足虽然不是一个特定的漏洞,但意味着存在高度可能存在其他未被发现的漏洞和错误。 它还会加剧复杂代码库中多个相互关联的风险因素。 这包括通常由测试提供的功能和精确预期行为的缺乏完整的、隐式的规范,这增加了错过正确性问题的可能性。 它还需要更多的努力来建立基本的正确性,并减少了探索边缘案例的努力,从而增加了错过复杂问题的可能性。
此外,缺乏对完整规范的重复自动化测试增加了引入重大更改和新漏洞的可能性。 这适用于先前经过审计的代码和未来对当前经过审计的代码的更改。 规范不足的接口和假设增加了细微集成问题的风险,而测试可以通过强制执行详尽的规范来减少这种风险。
为了解决这些问题,请考虑实施全面的多层测试套件。 这样的测试套件应包括合约级别的测试,覆盖率达到 95%-100%,按链/层部署,以及测试部署脚本和整个系统的集成测试,以及针对计划升级的每链/层 Fork 测试。 至关重要的是,测试套件应以这样一种方式进行文档化,以便审查人员可以独立于开发团队设置和运行所有这些测试层。 可以建议使用一些现有的此类设置示例作为后续对话中的参考。 此外,请考虑将所有测试套件合并为一个,以更好地维护。 实施这样的测试套件应具有非常高的优先级,以确保系统的稳健性并降低漏洞和错误的风险。
更新:已在 pull request #1038 中部分解决。 团队声明:
解决了以下 2 点:
- 新增内容仅向套件添加 5 个单一的积极整体案例,留下许多其他未经测试的边缘案例,并且没有断言任何负面情况。
- 用于测试输出费用的值已设置为零,因此绕过了整个费用保护。
_添加了多个新的本地(单元)测试用例(费用测试用例已作为修复问题 M-03 的一部分添加),以及一个通过
Universal_Adapter
发送 USDT 的 Fork 测试。 可以使用以下命令运行 Fork 测试:__
NODE_URL_1=<your-ethereum-rpc-url> forge test --match-path test/evm/foundry/fork/UniversalAdapterOFT.t.sol
_
在 OFTTransportAdapter
合约中,_transferViaOFT
函数检查在结束时收到的预期值是否与在来源处传递的输入相匹配。 在 LayerZero 中,本地小数位数的默认值为 18,但可以更改。 在这种情况下,amountReceivedLD
值将以目标处的本地小数位数表示,并且它将与以来源处的本地小数位数表示的 _amount
输入不匹配。 因此,传输将回滚,并且在该组合中使用 OFT 机制的移动将被停止。 请注意,如果两个链上的小数位数相同,则移动将起作用。
考虑考虑到两个链上的小数位数差异,并在验证收到的金额时执行转换。
更新:已确认,未解决。 在源和目标上实现不同的小数位数,因此偏离 默认实现 的资产可能会覆盖 OFTAdapter._debit()
和 OFTCore._debitView()
函数,从而导致 _transferViaOFT
函数中的验证回滚。 小数位数相同的 OFT 实现可能不会出现此问题。
团队声明:
_
_transferViaOft
流程:我们正在链上与 OFTAdapter 交互。 这是 默认实现。 我们正在调用(调用是针对继承OFTCore
的OFTAdapter
):__
OFTCore.send()
->OFTAdapter._debit()
->OFTCore._debitView()
__此调用链从 此处 开始。 在
_debitView
中,这是默认的 OFT 实现,我们看到 此:__
amountSentLD = _removeDust(_amountLD);
amountReceivedLD = amountSentLD;
_收发金额均使用*源链的本地小数位数(,因此小数位数差异在默认 OFT 实现中不会成为问题。USDT0 使用相同的底层逻辑(尽管它们的 合约 是可升级的)。
_总而言之,我们不希望在
_debit()
中支持具有非标准小数位数实现的代币,也不希望 OApp 在小数位数逻辑方面覆盖此函数。 我们可能期望某些 OApp 做的是可能覆盖_debit
在添加额外费用方面,我们目前还无法支持这些。_
__gap
变量的使用不一致在整个代码库中,发现了多个 __gap
变量的使用不一致的实例,这可能会导致安全和运营问题。
保留槽的数量似乎没有遵循特定的模式。 例如,在 没有定义本地存储变量的情况下,它已设置为 1000,但它 在已定义本地存储变量的情况下也为 1000。 这意味着保留槽和已用槽(即,由存储变量的定义使用)的总和不加起来为一个公共值,有时会大大超过其余合约的值。 例如,SpokePool
合约就是这种情况,它定义了多个存储变量,但也使用 __gap
变量保留了 997 个槽,导致存储槽的总值大于 1000。保持公共值的重要性在于,它可以帮助检查合约之间是否可能发生冲突,尤其是在它们相互继承时。
此外,没有适当的 文档来跟踪哪个槽代表哪个存储变量可能会导致使用的槽和定义的变量之间不匹配。 这可能会推动继承的存储布局并导致存储冲突。 此外,已经观察到,在 过去 中,对 __gap
变量进行了更改,这些更改 增加了其长度,同时 删除了一个存储变量。 这种更改是危险的,因为它们会创建一种情况,即在此类更改之后添加的新变量可能没有空值,并且可能会在漏洞利用中使用。
考虑审查整个协议以断言槽正在按计划使用,并在相应的 __gap
变量中全面记录变量及其槽。 此外,从现在开始,考虑标准化将用于未来合约的值。
更新:已在 pull request #1039 中解决。 团队声明:
_更新了
__gap
文档。_
当 to
地址被 SpokePool
合约的 _is7702DelegatedWallet
函数检测为 EIP-7702 EOA 钱包时,在发送 ETH 或 WETH 时,SpokePool
合约中的 _unwrapwrappedNativeTokenTo
函数将首先将 WETH 转换为 ETH,然后使用低级 .call
调用将其发送到钱包。 因此,此低级调用不会将 gas 限制为 2300,从而能够执行更复杂的操作。
此外,由于 EIP-7702 EOA 钱包可以使用任何实现,因此他们的 receive
或 fallback
方法可能会实现恶意逻辑,以便在不期望的情况下重新进入协议。
尽管多个函数都受到重入保护,但请考虑通过发送 WETH 将 EIP-7702 EOA 钱包视为合约来减少协议中的攻击面。 或者,考虑减少低级 .call
调用的 gas 津贴,使其无法执行任何存储更改或对另一个合约的外部调用。
更新:已确认,未解决。 团队声明:
填充功能已经在代码中的同一点支持具有无限 Gas 预算的外部调用。 从我们的角度来看,对 7702 钱包的任何 ETH 调用也可能在没有状态更改的情况下,通过
handleV3AcrossMessage
回调立即触发对合约的回调。 我们无法想到在一种情况下重入会成为问题,而在另一种情况下则不是。为了使协议能够以其当前的功能集运行,它必须通常具有弹性重入能力。 如果协议对重入没有弹性,那么我们认为应该以一种涵盖所有情况的方式来解决,而不仅仅是 ETH 到 7702 的情况。
我们不认为这些更改会影响协议的安全性,因此我们承认,但选择不进行建议的更改。
在 SpokePool
合约中,没有标准的方法来检查合约是否支持特定的接口或功能。 特别是,在添加消息传递者时,正在执行的 单次检查 是输入地址是否将其 token
函数与传递的函数匹配。 由于 token
函数在各种不同的合约中非常常见,因此可能会错误地传递一个实现 token
函数并检索特定代币的合约,但它不是 IOFT
类型的合约。 在这种情况下,分配将通过,但会导致意外情况。
在以太坊生态系统中,如果合约符合 EIP-165,则可以完成更彻底的验证。 接口是定义一组行为的函数集合。 通过实施标准,合约可以公开一个函数来查询它们是否支持特定接口,从而使其他合约更容易理解它们的功能。 由于消息传递者可能会升级,并且可以将新功能添加到现有合约,因此这种标准化还将允许新版本确保向后兼容性。
此外,IOFT
接口 定义了接口 ID,可以在设置消息传递者时使用它来补充此类验证。 另请注意,在 AdapterStore
合约中,通过 任何函数 添加的消息传递者没有像 [SpokePool
合约]中那样进行检查(https://github.com/across-protocol/contracts/blob/c5d7541037d19053ce2106583b1b711037483038/contracts/SpokePool.sol#L1740)。
考虑在与外部合约交互的模块中定义和实现标准接口检测机制,以通过为合约提供一种一致的方式来查询和识别它们支持的功能,从而提高其整体可用性、安全性和效率。 此外,考虑在 AdapterStore
合约中执行相同的验证,以与消息传递者的添加保持一致。
更新:已在 pull request #1033 中部分解决。 团队声明:
添加了建议更改的一部分:通过调用
.token()
在AdapterStore.sol
端进行 IOFT 验证。 这主要是为了保护人为错误。 实现 EIP-165 调用有点太多了。
在整个代码库中,发现了多个误导性文档的实例:
Universal_Adapter
合约中,内联文档声明relayTokens
函数_"仅使用 CircleCCTPAdapter
将 USDC 代币中继到启用 CCTP 的 L2 链" 。 虽然对于该代币来说部分正确,但新实现还允许使用 OFT 方法。AlephZero_SpokePool
合约中的 注释 声明"Arbitrum 仅支持 v0.8.19"。 但是,在 Arbiscan 上,更新的版本也 列为 受支持的版本。考虑更新文档以反映功能的当前行为。
更新:已在 pull request #1030 中部分解决。 仅 Universal_Adapter
合约已更新。
SpokePool
合约实现了跟踪各个 OFT 消息传递者与代币的基本原理。 它还继承了 OFTTransportAdapter
合约,并将其中的输入参数从 constructor
函数传递给它。 目前,只有基于 Arbitrum 的合约(参见 AlephZero)和通用 SpokePool
和适配器使用了此类 OFT 功能。 这意味着在任何其他链上,这些 constructor
参数都 设置为零。
这种方法引发了以下问题:
_transferViaOFT
函数 未在此类适配器或 SpokePool
的实现中调用,并且可能 未设置消息传递者和/或代币,在 OFT_DST_EID
和 OFT_FEE_CAP
参数 中使用零值也可能会增加攻击面,因为它们 被用作检查。 这可能会在未来的升级中引起潜在问题,其中流程可能会达到这样的调用并使其成为漏洞利用的一部分。考虑在不使用适配器和 SpokePool
时限制 OFT 继承的功能,以防止上述情况并减少攻击面。
更新:已确认,未解决。 团队声明:
_拥有
SpokePool
继承OFTTransportAdapter
使我们能够更轻松地向更多的轮辐添加 OFT 功能。 此外,OFT_FEE_CAP
旨在防止发送过多的费用。 它不负责针对其他场景的保护。 设置为0
,它仍然履行其职责。 防止错误设置 OFT 消息传递者等的保护取决于管理员的诚实。_
abstract
合约中的 internal
和 public
状态变量允许子级合约直接修改它们。 这可能会破坏状态变量的预期属性,并由于缺少变量更改的事件发出而限制链下监控功能。
具体来说,在 SpokePool.sol
中,SpokePool
抽象合约包含 oftMessengers
状态变量,它是 public
。 此外,由于 oftMessengers
映射仅由 SpokePool
合约内的设置器和获取器访问,因此没有必要在子级合约中保持如此高的可见性。
考虑对抽象合约中的状态变量使用 private
可见性。 此外,考虑创建 internal
函数来更新这些变量,这些函数发出适当的事件,并验证是否满足了理想条件。
更新:已确认,未解决。 团队声明:
拟议的解决方案带来了一个限制,即无法在 Etherscan 等上看到设置的消息传递者。 虽然可以通过在 SpokePool 上添加额外的公共获取器来缓解这种情况,但我们认为 SpokePool 代码已经很复杂,并且添加这样的代码可能会使尝试更多地了解它的人感到不知所措。 我们希望保持
oftMessengers
的当前可见性
当使用地址参数执行操作时,确保该地址未设置为零至关重要。 将地址设置为零是有问题的,因为它具有特殊的销毁/放弃语义。 此操作应由单独的函数处理,以防止在值或所有权转移期间意外丢失访问权限。
在整个代码库中,有多个操作缺少零地址检查的实例:
AdapterStore.sol
AdapterStore
合约中的 _setMessenger(messengerType, dstDomainId, srcChainToken, srcChainMessenger)
操作。AdapterStore.sol
AdapterStore
合约中的 _setMessenger(messengerTypes[i], dstDomainIds[i], srcChainTokens[i], srcChainMessengers[i])
操作。OFTTransportAdapterWithStore.sol
OFTTransportAdapterWithStore
合约中的 _adapterStore
操作。SpokePool.sol
SpokePool
合约中的 _setOftMessenger(token, messenger)
操作。考虑在分配状态变量之前添加零地址检查。
更新:已确认,未解决。 团队声明:
_经过内部讨论,我们认为此处的零地址检查有点过分,因为所有提到的函数只能由管理员调用(
_adapterStore
情况除外,但那是合约创建,并且此合约只能在管理员操作后在系统中使用)_
在整个代码库中,发现了多个缺少 docstrings 的实例。 特别是在以下文件中:
AdapterStore.sol
Arbitrum_Adapter.sol
Arbitrum_SpokePool.sol
OFTTransportAdapter.sol
OFTTransportAdapterWithStore.sol
Ovm_SpokePool.sol
PolygonZkEVM_SpokePool.sol
Polygon_SpokePool.sol
Scroll_SpokePool.sol
SpokePool.sol
Succinct_SpokePool.sol
Universal_Adapter.sol
Universal_SpokePool.sol
ZkSync_SpokePool.sol
考虑彻底记录所有属于任何合约的公共 API、事件、存储变量和常量的函数(及其参数)。 即使未公开,实现敏感功能的函数也应清楚地记录下来。 编写 docstrings 时,请考虑遵循 Ethereum Natural Specification Format (NatSpec)。
更新:在 84f87be 提交中部分解决。 团队声明:
向以下项添加了 docstrings:
-
AdapterStore.sol
-OFTTransportAdapter.sol
-OFTTransportAdapterWithStore.sol
保持对 OFT 范围的更改。
应该固定 Pragma 指令,以清楚地标识将使用其编译合约的 Solidity 版本。
在整个代码库中,发现了多个浮动 pragma 指令的实例:
AdapterStore.sol
具有 solidity ^0.8.0
浮动的 pragma 指令。AlephZero_SpokePool.sol
具有 solidity ^0.8.19
浮动的 pragma 指令。Arbitrum_Adapter.sol
具有 solidity ^0.8.0
浮动的 pragma 指令。Arbitrum_SpokePool.sol
具有 solidity ^0.8.0
浮动的 pragma 指令。Ethereum_SpokePool.sol
具有 solidity ^0.8.0
浮动的 pragma 指令。Linea_SpokePool.sol
具有 solidity ^0.8.19
浮动的 pragma 指令。OFTTransportAdapter.sol
具有 solidity ^0.8.0
浮动pragma 指令。OFTTransportAdapterWithStore.sol
具有 solidity ^0.8.0
浮动 pragma 指令。Ovm_SpokePool.sol
具有 solidity ^0.8.0
浮动 pragma 指令。PolygonZkEVM_SpokePool.sol
具有 solidity ^0.8.0
浮动 pragma 指令。Polygon_SpokePool.sol
具有 solidity ^0.8.0
浮动 pragma 指令。Scroll_SpokePool.sol
具有 solidity ^0.8.0
浮动 pragma 指令。SpokePool.sol
具有 solidity ^0.8.0
浮动 pragma 指令。Succinct_SpokePool.sol
具有 solidity ^0.8.0
浮动 pragma 指令。Universal_Adapter.sol
具有 solidity ^0.8.0
浮动 pragma 指令。Universal_SpokePool.sol
具有 solidity ^0.8.0
浮动 pragma 指令。ZkSync_SpokePool.sol
具有 solidity ^0.8.0
浮动 pragma 指令。考虑使用固定的 pragma 指令。
更新: 已确认,未解决。 团队声明:
经过内部讨论,我们得出结论,不希望将迁移到固定的 pragma 作为本次审计的一部分。 也许在未来! 感谢你的建议。
为了清楚地识别合约将使用哪个 Solidity 版本进行编译,pragma 指令应在所有文件导入中保持固定和一致。
在整个代码库中,发现了多个不同的 pragma 指令的实例:
AlephZero_SpokePool.sol
具有 pragma solidity ^0.8.19;
pragma 指令,但导入了 Arbitrum_SpokePool.sol
,后者具有不同的 pragma 指令。Linea_SpokePool.sol
具有 pragma solidity ^0.8.19;
pragma 指令,但导入了 SpokePool.sol
,后者具有不同的 pragma 指令。考虑在所有文件中使用相同的、固定的 pragma 指令。
更新: 已确认,未解决。 团队声明:
经过内部讨论,我们得出结论,不希望将迁移到固定的 pragma 作为本次审计的一部分。 也许在未来! 感谢你的建议。
L1 上的 Arbitrum_Adapter
合约正被 HubPool
合约使用,后者 delegateCall
了它的执行。 所有传递给构造函数的参数都用于设置 immutable 变量,这意味着这些变量稍后可以在 delegateCall
期间使用。 然而,这也为某人创造了使用 adapter 作为系统入口点而不是通过 HubPool
合约的可能性,因为所有参数也在那里设置。 即使在正常情况下 Arbitrum_Adapter
合约不应有任何资产,但如果用户错误地将资产发送给它,另一个参与者可能会抓住机会将这些资产中继到个人地址。
为了防止上述情况发生,请考虑强制执行该实现在被直接调用时不能使用。
更新: 已确认,未解决。 团队声明:
Adapter 的实现不应该持有任何资产。 我们将来可能会考虑这个问题,但不会作为本次审计的一部分。
为了防止通过 OFT 消息系统发送具有不合理费用的 token,OFTTransportAdapter
合约对费用施加了一个上限,并且这些费用作为 immutable 的参数存储。 此合约正被用于adapter 和 SpokePool
合约。 然而,如果消息费用超过上限,所有 OFT 转账都将失败。 由于上限的 immutable 性质,没有简单的方法可以进一步增加它来解决问题。 这将需要部署一个新的 adapter 并使用 HubPool
合约中的 setCrossChainContracts
函数设置它,或者部署一个新的具有新值的 SpokePool
合约并执行升级。
由于上述两种解决费用超过上限的方法都可能在成本波动期间使协议无法使用,请考虑实施修改上限的功能。 或者,考虑彻底记录一个应急计划,以便在这种情况出现后尽快解决。
更新: 已确认,未解决。 团队声明:
"这将需要部署一个新的 adapter 并使用
HubPool
合约中的setCrossChainContracts
函数设置它,或者部署一个新的具有新值的SpokePool
合约并执行升级。" - 这是我们更新这些参数的标准方法,是的。我们的部署设置了 1 ETH 的高上限,因此我们预计这不会成为问题。 更重要的是,失败的 OFT 转账不会导致系统无法使用。 它可能会延迟 bundle 或 leaf 的执行,这可能会延迟中继者的偿还。 用户存款和充值仍将按预期工作,因此我们有时间对这个问题做出反应(如果出现)。
OFTTransportAdapter
合约的 _transferViaOFT
函数允许在链之间使用 OFT 传输。 为了确认资产已正确到达,该函数实现了一个比较从一条链发送的金额与在另一条链上收到的金额的检查。 但是,如果底层 token 在转账时收取费用,则这两个金额将不同(即,输入 _amount
将与输出 amountReceivedLD
不匹配)。
考虑实施必要的逻辑以允许转账 fee-on-transfer token。 或者,考虑记录以下事实:启用费用后,fee-on-transfer token 不能与 OFT 转账一起使用。
更新: 已确认,未解决。 团队声明:
是的,没关系。 我们计划只支持没有转账费用的信誉良好的 token。
在 OFTTransportAdapter
合约的 _transferViaOFT
函数中,最终验证检查作为输入传递的资产金额与将在目的地收到的金额是否匹配。 但是,在转账期间调用 messenger 时,将调用 LayerZero 中的 _removeDust
函数,它将从输入金额中删除所有无法通过 sharedDecimals
值(默认情况下为 6)表示的数字。 这意味着,如果 _amount
输入在 decimalConversionRate
值的整数除法之后有剩余,则 amountReceivedLD
值将比 _amount
值小 dust,因此两者将不匹配。
相关地,有一个内联注释表明 _"将 minAmountLD
设置为等于 amountLD
可保护我们免受由于内部 OFT 合约逻辑(例如 _removeDust
)导致的发送金额的任何更改" _,指的是两个值相同且等于发送的金额,即 _amount
输入。 但是,值得注意的是,此措施不能提供更大的保护以防止 dust 移除,因为后者是通过最终检查 oftReceipt
输出来完成的。 实际上,此检查仅使用发送的金额(_amount
),而不使用最小金额,但即使不使用最小金额执行验证,dust 移除保护仍然会被 oftReceipt
输出的最终检查捕获。 即使该金额在提交 bundle 时由 Dataworker 传递,也应注意,输入金额可以在通过 OFT messenger 发送之前进行转换,以便它首先不包含任何 dust,从而最大限度地减少 revert 的情况。
考虑实施必要的计算和转换以防止 dust revert 交易。 特别是,该金额可以四舍五入到 decimalConversionRate
值给出的下一个精度,以防止发送少于 Dataworker 传递的资产。
更新: 已确认,未解决。 团队声明:
这是我们采用的设计决策:我们已将此舍入要求添加到相关的 UMIP,作为 bundle 正确性的要求。 因此,dataworker 负责提供正确的 decimals。 否则,如果 dataworker 代码中存在错误,OFT 发送确实会 revert;这是期望的行为。
Pull request 1031 删除了已经宣布的已弃用功能,例如 depositDeprecated_5947912356
和 depositFor
函数,以及它们的 _deposit
内部函数。
但是,由于这些入口点已被删除,代码库的未来版本可能会引入与已删除的函数具有相同函数选择器的新函数。 在这种情况下,可能使用了已弃用功能的协议现在可以调用到新的功能,并产生意想不到的结果。
同样,如果将来使用 fallback
函数,具体取决于其实施,它可以使用使用旧的已弃用功能的协议的 calldata,并产生类似的结果。
为了防止重复使用已弃用的函数选择器,请考虑保留公共函数的声明,而无需其原始定义,并 revert 对它们的调用。
更新: 已在提交 0e262bf
的 pull request #1048 中解决。
SpokePool
中的零地址验证绕过SpokePool
合约的 _setOftMessenger
函数执行检查以验证提议的 OFT messenger 合约对于给定的 token 是否合适。 但是,如果 messenger 尚未设置其 token,则管理员能够将 messenger 链接到零地址,因为 _token
参数。 即使对该零地址 token 的连续调用将在 OFT 传输流程中失败,也建议减少边缘情况的攻击面。
考虑始终验证 token 地址输入不是零地址。
更新: 已确认,未解决。 团队声明:
没关系,我们相信管理员不会这样做。
印刷错误会降低可读性并可能导致误解。 在整个代码库中,发现了多个印刷错误的实例:
OP_Adapter.sol
中,第 41 行 中的文档声明为 "Desination",而应该是 "Destination"。OFTTransportAdapter.sol
中,第 54 行 中的文档声明为 "trasnfer",而应该是 "transfer"。考虑更正任何印刷错误的实例,并使用拼写检查工具以避免再次发生。
更新: 已在 pull request #1035 中解决。
当 setter 函数不检查该值是否已更改时,它会创建 spam 事件的可能性,这些事件表明该值已更改,即使它没有更改。 重复 spam 相同的值可能会混淆链下客户端。
在 SpokePool
合约中,当设置新的 messenger时,新值可以与存储中的值相同,从而允许通过设置相同的值多次触发相同的事件。
考虑添加一个检查,如果正在设置的值与现有值相同,则 revert 交易。
更新: 已确认,未解决。 团队声明:
我们依靠管理员来不生成重复的 "set 事件"。
当状态变量在合约中使用 public
可见性时,会自动包含该变量的 getter 方法。
在 SpokePool
合约中,_getOftMessenger
函数 是多余的,因为 oftMessengers
状态变量 已经有一个 getter。
为了提高代码库的整体清晰度、意图和可读性,请考虑删除任何冗余的 getter 函数或记录保留它们的原因。
更新: 已确认,未解决。 团队声明:
我们认为从子合约中使用原始
oftMessengers
映射比使用 getter 更危险(程序员更容易犯错)。
在整个代码库中,发现了多个函数的可见性不必要地宽松的实例:
SpokePool
合约中具有 internal
可见性的 _setOftMessenger
函数可以限制为 private
。SpokePool
合约中具有 public
可见性的 setOftMessenger
函数可以限制为 external
,因为它必须由管理员调用。为了更好地传达函数的预期用途,并有可能实现一些额外的 Gas 节省,请考虑将函数的可见性更改为仅所需的那样宽松。
更新: 已在 pull request #1041 中解决。
在代码库中使用非显式导入会降低代码清晰度,并可能在本地定义和导入的变量之间创建命名冲突。 当同一 Solidity 文件中存在多个合约或继承链较长时,这尤其重要。
在整个代码库中,发现了多个非显式/全局导入的实例:
SpokePool.sol
中的 import "./libraries/OFTTransportAdapter.sol"; 导入Universal_Adapter.sol
中的 import "../libraries/OFTTransportAdapterWithStore.sol"; 导入遵循代码越清晰越好的原则,请考虑使用命名导入语法 (import {A, B, C} from "X"
) 来显式声明要导入的合约。
更新: 已在 pull request #1036 中解决。
在 AdapterStore.sol
中,已经声明了多个合约或库。 这些是 MessengerTypes
库 和 AdapterStore
合约。
考虑将合约分离到它们自己的文件中,以使开发人员和审阅者更容易理解代码库。
更新: 已确认,未解决。 团队声明:
由于
MessengerTypes
lib 非常小,我觉得它提高了可读性,而不是降低了可读性。
自 Solidity 0.8.18 以来,开发人员可以在映射中使用命名参数。 这意味着映射可以采用 mapping(KeyType KeyName? => ValueType ValueName?)
的形式。 此更新的语法提供了映射用途的更透明的表示。
在整个代码库中,发现了多个没有命名参数的映射的实例:
AdapterStore
合约中的 crossChainMessengers
状态变量SpokePool
合约中的 oftMessengers
状态变量考虑将命名参数添加到映射中,以提高代码库的可读性和可维护性。
更新: 已在 pull request #1040 中解决。
在智能合约中提供特定的安全联系人(例如电子邮件或 ENS 名称)可以大大简化个人在代码中发现漏洞时进行沟通的过程。 这种做法非常有益,因为它允许代码所有者指定漏洞披露的沟通渠道,从而消除了由于缺乏如何操作的知识而导致沟通不畅或未能报告的风险。 此外,如果合约包含第三方库并且这些库中出现错误,则其维护者可以更轻松地联系相应人员以了解问题并提供缓解说明。
在整个代码库中,发现了多个缺少安全联系人的合约实例:
考虑在每个合约定义上方添加一个包含安全联系人的 NatSpec 注释。 建议使用 @custom:security-contact
约定,因为它已被 OpenZeppelin Wizard 和 ethereum-lists 采用。
更新: 已在 pull request #1037 中解决。
require
语句中的自定义错误自从 Solidity 版本 0.8.26
以来,自定义错误支持已添加到 require
语句。 最初,此功能仅通过 IR 管道提供。 但是,Solidity 0.8.27
也将其支持扩展到旧版管道。
在整个代码库中,发现了多个可以用 require
语句替换 if-revert
语句的实例:
OFTTransportAdapter.sol
中的 if (nativeFee > OFT_FEE_CAP) revert OftFeeCapExceeded()
语句OFTTransportAdapter.sol
中的 if (nativeFee > address(this).balance) revert OftInsufficientBalanceForFee()
语句OFTTransportAdapter.sol
中的 if (_amount != oftReceipt.amountReceivedLD) revert OftIncorrectAmountReceivedLD()
语句SpokePool.sol
中的 [if (IOFT(_messenger).token() != _token) {\\ revert OFTTokenMismatch();\\ }
](https://github.com
- 原文链接: blog.openzeppelin.com/ac...
- 登链社区 AI 助手,为大家转译优秀英文文章,如有翻译不通的地方,还请包涵~
如果觉得我的文章对您有用,请随意打赏。你的支持将鼓励我继续创作!