Across协议 SVM Solidity审计

本文是对Across协议的Solidity合约进行的安全审计报告,重点关注了为支持Solana网络和ERC-7683订单所做的代码更改。审计发现了包括潜在的资金池耗尽漏洞、地址转换错误、重复代码、缺少单元测试等问题,并提出了修复建议。报告还包括对之前提交的PR的审查以及客户端报告的问题。

目录

总结

TypeDeFiTimeline从 2024-12-12 到 2024-12-20 语言 Solidity 问题总数 11 (10 已解决) 严重程度的问题 1 (1 已解决) 高严重程度的问题 0 (0 已解决) 中等严重程度的问题 0 (0 已解决) 低严重程度的问题 5 (4 已解决) 注释 & 附加信息 3 (3 已解决)

范围

我们的目标是 across-protocol/contracts 仓库,有三个不同的目标:

1) 在 master 分支的 commit 7f0cedb 中,我们审查了 ERC-7683 的实现,重点关注最早在 pull request #639 中引入的可预测中继哈希功能。

2) 在 svm-dev 分支的 commit 401e24c 中,我们审计了对以下文件所做的更改:

 contracts/
├── SpokePool.sol
├── SpokePoolVerifier.sol
├── SwapAndBridge.sol
├── erc7683/
│   ├── ERC7683OrderDepositor.sol
│   ├── ERC7683OrderDepositorExternal.sol
│   ├── ERC7683Permit2Lib.sol
├── external/interfaces/
│   ├── CCTPInterfaces.sol
├── interfaces/
│   ├── V3SpokePoolInterface.sol
├── libraries/
│   ├── AddressConverters.sol
│   ├── CircleCCTPAdapter.sol
├── permit2-order/
│   ├── Permit2Depositor.sol

3) 在 pull request #805 中,作为此审计的修复审查过程的一部分,我们审查了这些更改。

系统概述

该协议支持在多个区块链网络之间即时传输代币。协议的核心是以太坊主网上的 HubPool 合约,它是系统中所有合约的中心流动性枢纽和跨链管理员。该池管理部署在各个网络上的 SpokePool 合约,这些合约要么发起代币存款,要么充当传输的最终目的地。有关协议及其各种组件的更详细概述,请参阅我们之前的报告。

经过审计的更改主要围绕着增加与 Solana 网络的兼容性。为了实现这一点,进行了以下更改:

  • 许多 address 数据类型已转换为 bytes32,以使其更通用并与 Solana 兼容。
  • fill 函数中添加了一个 repaymentAddress 参数,以便中继器可以指定不同的地址。
  • 存在一个边缘情况,其中具有黑名单的代币可能会阻止执行与退款相关的传输。因此,bundle 中恢复的传输现在被计入一个新的映射中,该映射允许中继器检索无法作为正常中继器退款调用的一部分传输的退款。

为 Solana 网络添加兼容性还包括其他的拉取请求和文件。但是,对于本次审计,我们仅专注于对上述范围中提到的现有 Solidity 文件所需的更改。

严重级别 - 严重

任何人都可以锁定中继器退款,并且合约可能被耗尽

claimRelayerRefund 函数 旨在让中继器可以选择声明未偿还的退款。这可能会发生在不同的边缘情况下,例如黑名单不允许代币转账。在这种情况下,中继器可以调用此函数并指定不同的 refundAddress 来获得其资金。

该函数首先使用 l2TokenAddressmsg.sender 键从 relayerRefund 映射中读取当前未偿还的退款。如果此值大于 0,则将其转移refundAddress 并发出适当的事件。

在转出代币之前,为了正确的核算,映射值被设置为 0。但是,用于重置映射值的键是 refundAddress 而不是 msg.sender。这为任何拥有少量退款金额的中继器打开了方便之门,他们可以通过指定其 refundAddress 将任何其他中继器的退款归零,从而有效地使中继器失去退款。此外,由于原始映射值从未重置,因此恶意中继器可以通过重复调用该函数来耗尽 l2TokenAddress 合约的整个余额来利用此漏洞。

考虑使用正确的 msg.sender 键而不是 refundAddress 来正确地将退款金额设置为零。

更新:pull request #826 中已解决。该代码现在可以正确重置映射值。

严重级别 - 低

函数可以声明为 external

SpokePool 合约的 claimRelayerRefund 函数 被声明为 public,但可以改为定义为 external

为了提高整体代码质量并提高 gas 消耗,请考虑将函数可见性更改为 external

更新:pull request #827 中已解决。

_destinationSettler 可能返回零地址

ERC7683OrderDepositor 合约的 _resolveFor 函数中,从 internal _destinationSettler 函数返回的值未经过验证是否为非零地址_destinationSettler 函数返回目标链的 settler 合约。如果此值尚未设置,则将返回默认值并将其传递给 resolvedOrder 参数的 fillInstructions 字段。

考虑添加 resolvedOrder 的 FillInstruction 结构体中使用的 _destinationSettler 值不能为零地址的要求。

更新:pull request #834 中已解决。

AddressConverters 库中不正确的右移

AddressConverters 库的 toAddress 函数向右移动 192 位。但是,它应该向右移动 160 位,以便正确检查 _bytes32 的高 96 位是否确实为空。

考虑更新 toAddress 函数,使其向右移动 160 位。

更新:pull request #828 中已解决。

重复的函数

ERC7683OrderDepositor 合约的 _toBytes32 internal 函数 与导入的 AddressConverters 库的 toBytes32 函数 具有相同的逻辑。_toBytes32toBytes32 函数都在整个 ERC7683OrderDepositor 合约中使用 ,这降低了代码的清晰度。

为了提高整体代码质量并避免重复代码,请考虑删除 _toBytes32 函数并改用库函数。

_更新:pull request #829 中已解决。该团队还删除了内部 _toAddress 函数,转而使用同一库文件中存在的等效函数。_

缺乏 ERC-7683 的单元测试

随着协议的扩展,维护一个涵盖所有新功能的综合测试套件至关重要。目前,该仓库不包含任何针对 ERC7683OrderDepositorERC7683OrderDepositorExternal 合约的单元测试。

为了提高代码库的健壮性和安全性,请考虑为 EIP-7683 的实现实施一个具有高度覆盖率的综合测试套件。

更新: 已确认,将解决。该团队计划将更改集成到测试和覆盖率中,问题将随之解决。

注释 & 附加信息

缺少文档字符串

缺少文档字符串会对代码的清晰度和可维护性产生负面影响。在 SpokePool 合约的 fillV3RelayWithUpdatedDepositfillV3Relay 函数的文档字符串中未记录 repaymentAddress 参数。

考虑为所有输入参数添加文档字符串。

更新:pull request #830 中已解决。

拼写错误

在整个 SpokePool 合约中,发现了多个拼写错误实例:

考虑修复所有拼写错误,以提高整体代码的清晰度。

更新:pull request #831 中已解决。

拉取请求 #805 的审查

如范围部分所述,在对当前审计进行修复审查时,我们还审查了拉取请求 #805。该拉取请求引入了对事件和函数名称的更改以及其他一些小更改。

我们从此类审查中强调的注释如下:

  • SpokePool 合约的 fill 函数已从使用 abi.encodeCall 恢复为使用 abi.encode,这在类型上不安全。我们鼓励保持类型安全性,并改用 encodeCall
  • SpokePool 合约中,pauseDepositsnatspec 应包括 speedUpDeposit(),而不是 speedUpV3Deposit()
  • 在新的 fillV3Relay 中,采用了 msg.sender,而在 fillRelay 中,它是一个指定的 repaymentAddress。考虑在这些函数中保持一致的行为。
  • 鉴于拉取请求评论中激增的对话,我们还鼓励团队增加测试覆盖率,重点关注向后兼容性。

更新: 已解决,该团队在同一拉取请求的 commit a28eb83 中修复了前两点。该团队还告知我们,他们正在扩展测试套件。关于第三项,该团队表示:

不添加此道具的原因是保持接口与当前 fillV3Relay 方法等效,如此处定义。在当前实现中,此方法默认将 filler (msg.sender) 设置为 repayment 地址。这确保了此方法与之前的 fillV3Relay 方法保持向后兼容。

客户端报告

重复的填充执行

客户端报告了一个严重级别的问题,该问题将允许执行有效的重复填充,并可能导致协议损失资金。

Across 中的存款使用 V3RelayData 结构的哈希值进行唯一标识。对加粗代码库的拟议更改引入了对 _getV3RelayHash 函数的更改。该更改修改了计算哈希值的方法,仅包括 V3RelayData 结构体中包含的 message 字段的哈希值,而不是对整个结构体进行哈希处理。

问题在于,实际上只能异步进行 SpokePool 合约的升级。filledStatuses 映射使用中继数据的哈希值来记录订单的状态。由于不同版本之间的哈希值派生方法不同,这意味着同一个订单将存在两个有效的哈希值,一个是在升级之前,另一个是在升级之后,这意味着检查确认订单是否已填充将引用不同的哈希值,因此对于已填充的订单,将无法恢复。

这与集成到协议中的链下组件具有关键的交互作用。如果中继器或捆绑器使用中继数据的哈希值作为唯一标识符来跟踪存款(而不是使用每个存款的 depositId),那么由于哈希值不同,这些参与者将无法识别订单是否已填充。因此,如果订单被填充两次,协议和/或中继器可能会损失资金,因为只会收到一笔存款。

更新:已解决。客户端已通过恢复对哈希值派生的拟议更改来修复此问题。

缺少有效的地址检查

SpokePool 合约的 deposit 函数中,没有检查为 bytes32 参数提供的值是否为有效的 EVM 地址。例如,如果意外提交格式不正确的存款人地址,则在未填充的情况下将无法退款。

更新: 已解决。客户端通过引入针对存款人参数的有效地址检查解决了该问题。我们鼓励团队在所有打算将 bytes32 值用作地址的实例中重复执行此操作。

结论

Across 协议支持在以太坊 L1 和 L2 链之间快速传输代币。用户存入资金,中继器将资金转移到目的地,存款人收到扣除费用的金额。L1 上的 HubPool 合约管理流动性,并与每个受支持链上的 SpokePool 合约协调。

现在,Across 协议的 Solidity 合约已更新为支持 Solana,并为 ERC-7683 订单提供支持。本次差异审计中审查的更改表明,在对支持其他链的需求可能增加的情况下,对代码库架构进行了周全的考虑。我们鼓励团队扩展测试套件,以包括对最终的 ERC-7683 实现的特定测试。

我们要感谢 Risk Labs 团队愿意回答问题,并在整个审计过程中提供有用的背景信息。

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

0 条评论

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