OpenZeppelin
Skip to content

Across V3 Incremental Audit

OpenZeppelin Security | February 21, 2024

Table of Contents

Summary

Type
Bridge
Timeline
From 2024-01-08
To 2024-01-30
Languages
Solidity
Total Issues
27 (25 resolved)
Critical Severity Issues
0 (0 resolved)
High Severity Issues
0 (0 resolved)
Medium Severity Issues
4 (3 resolved)
Low Severity Issues
7 (7 resolved)
Notes & Additional Information
16 (15 resolved)

Scope

We audited the diff of the UMAprotocol/across-contracts-v2-private repository from the base at commit b41fcee and the head at commit 8595081.

In scope were the following files:

 contracts
├── Arbitrum_SpokePool.sol
├── Base_SpokePool.sol
├── Ethereum_SpokePool.sol
├── Linea_SpokePool.sol
├── MerkleLib.sol
├── Optimism_SpokePool.sol
├── Ovm_SpokePool.sol
├── PolygonZkEVM_SpokePool.sol
├── Polygon_SpokePool.sol
├── Scroll_SpokePool.sol
├── SpokePool.sol
├── Succinct_SpokePool.sol
├── SwapAndBridge.sol
├── ZkSync_SpokePool.sol
├── chain-adapters
│   ├── Arbitrum_Adapter.sol
│   ├── Base_Adapter.sol
│   ├── CrossDomainEnabled.sol
│   ├── Linea_Adapter.sol
│   ├── Optimism_Adapter.sol
│   ├── PolygonZkEVM_Adapter.sol
│   ├── Polygon_Adapter.sol
│   └── Scroll_Adapter.sol
├── external
│   └── interfaces
│       └── CCTPInterfaces.sol
├── interfaces
│   └── SpokePoolInterface.sol
├── libraries
│   └── CircleCCTPAdapter.sol
└── permit2-order
    ├── Permit2Depositor.sol
    ├── Permit2Order.sol
    └── Permit2OrderLib.sol

We also audited pull request #1 and in scope were the following files:

 contracts
├── MerkleLib.sol
├── Ovm_SpokePool.sol
├── Polygon_SpokePool.sol
├── SpokePool.sol
├── SpokePoolVerifier.sol
└── interfaces
    ├── SpokePoolInterface.sol
    └── V3SpokePoolInterface.sol

Additionally, we audited pull request #29 and in scope were the following files:

 contracts
├── MerkleLib.sol
├── Polygon_SpokePool.sol
├── SpokePool.sol
└── interfaces
    └── V3SpokePoolInterface.sol

System Overview

The Across protocol is a cross-chain bridge that allows for quickly bridging ERC-20 tokens from any supported origin chain to any supported destination chain. Users of the protocol, called depositors, deposit their funds into the protocol on the origin chain. Then, they wait for the relayers to fill the deposits on the destination chain, using their own funds, by sending the specified amount minus fees. The protocol refunds the relayers by sending funds available on the chain of relayers' choice or by tapping into the HubPool on Ethereum if a specific chain does not have enough funds. More information about the protocol can be found in one of our previous reports.

Across V3 Upgrade

The main changes going from V2 to V3 version of the protocol include changes to the smart contract interface, the addition of new features, and the removal of some of the existing features. The input and output tokens can now be different. While the output token can be virtually any token, the input token can only be one whitelisted by the protocol. As a result, depositors are responsible for calculating the value of both tokens and setting the input and output amounts such that they incentivize relayers to fill the deposit. Relayers on their side should assess whether filling a particular deposit is profitable for them.

Both relayer and protocol fees are now implied by the spread between the input and output amounts. The protocol fee is taken as part of the relayer fee. This replaces the realized liquidity provider fee parameter which, however, is still present in the V2 deposits for backwards compatibility. Moreover, two new parameters were added to deposits. The first one is the "fill deadline" after which the deposit is not fillable. If a deposit is not filled and the fill deadline has passed then the refund process is triggered. This process uses the same refund mechanism as relayer refunds. Thus, the relayer refund roots might contain both the depositor refund leaves and relayer refund leaves.

The second one is the "exclusive relayer" which is the only address that can fill the deposit up until the "exclusivity deadline". The exclusivity deadline can be earlier than the fill deadline in which case the deposit can still be filled after the exclusivity deadline and up until the fill deadline. Partial fills are not possible for V3 deposits but still possible for remaining V2 deposits.

We would also like to highlight some of the less obvious consequences of the upgrade:

  • Slow fills can happen only if the input and output tokens are equivalent. The equivalence is determined by the configurable mapping in the protocol. For each token, there can be only one equivalent token per chain.
  • Regardless of whether the tokens are equivalent or not, the refund process takes place in case a deposit has expired. This ensures that all deposits are eventually either filled or refunded.
  • Refunds are issued only in the input tokens which are whitelisted by Across. Both depositor and relayer refunds share the same refund root.
  • Slow fills must be explicitly requested via a designated function.

Upgrade Process

The upgrade will happen in two stages. Deployment of the first stage adds the V3 interface and refactors V2 deposit functions. Both interfaces will co-exist with V2 deposits emitting V3 events. This ensures graceful migration from V2 to V3 because no new V2 deposits are created but the existing V2 deposits can still be filled and refunded using V2 functions. Once all the V2 deposits are filled, all V2 refunds are executed, and all parties have migrated to the V3 interface, the second stage will be deployed and the V2 functions will be removed.

Swap and Bridge

Even though V3 deposits allow specifying virtually any token as the output token, the input token can only be one of the tokens whitelisted by the protocol. Some depositors might not have one of the whitelisted tokens but may still want to use the protocol. To overcome this limitation, the SwapAndBridge periphery contract was added to facilitate bridging from any token to any other token. It transfers a depositor-specified token from the depositor to itself, performs a swap to an input token, and makes an AcrossV3 deposit on behalf of the depositor. The DEX used to perform the swap is set by the deployer at deploy time.

Support for Cross-Chain Transfer Protocol

The Cross-Chain Transfer Protocol (CCTP) is now used to bridge USDC on Arbitrum, Base, Optimism, and Polygon. While this speeds up the USDC bridging between the L2 chains and the Ethereum Mainnet, it makes the protocol rely on the CCTP infrastructure for functioning which introduces new trust assumptions. In particular, rebalancing, which moves USDC from SpokePools to the HubPool and vice versa, is now faster on the aforementioned chains.

Support for Linea, Polygon zkEVM, and Scroll Chains

A pair of SpokePool and adapter contracts was added for Linea, Polygon zkEVM, and Scroll chains to support them in the protocol. Below, we briefly describe the workings of the protocol on each of these chains.

On Linea, WETH is bridged as ETH via the canonical message service. USDC is bridged via its own Linea-specific bridge which is not CCTP. Other tokens are bridged via the canonical token bridge. On Polygon zkEVM, all tokens are bridged via the canonical bridge but WETH has a separate procedure which bridges it as ETH. On Scroll, all tokens are bridged via the canonical gateway router.

Integration with Third-Party Limit Order Protocol and Permit2

The Permit2Depositor periphery contract was added to translate limit orders of the third-party limit order protocol to cross-chain limit orders. The third-party protocol collects a user's permit2 signatures and makes them available for anyone to deposit on behalf of said user via the Permit2Depositor contract. In the process, the input tokens are pulled from the user and the one who deposits on behalf of the user via the Permit2 contract. The latter serves as collateral and is returned in the relayer refund process. An Across deposit is then created for the input token amount plus the collateral amount with the user as the depositor and the one who deposits on behalf of the user as the exclusive relayer.

Other Changes

  • Some of the SpokePools' storage variables were made immutable for gas savings. In particular, the _wrappedNativeTokenAddress and _depositQuoteTimeBuffer variables are now immutable.
  • The _preExecuteLeafHook internal function was added. It executes a SpokePool-specific hook before a refund or slow fill leaf execution.
  • Refund leaf execution which bridges funds from the Polygon SpokePool to the HubPool now must be the only function call in the transaction.

Security Model and Trust Assumptions

Here we present changes to the security model and new trust assumptions. You can find more information about the security model, trust assumptions, and privileged roles in one of our previous reports.

Bridge Infrastructure Assumptions

The protocol generally relies on canonical bridges. However, with the introduction of CCTP, USDC bridging now relies on the CCTP infrastructure. This exposes the protocol to an additional risk of infrastructure failure.

ERC-20 Approvals

The SwapAndBridge contract holds users' approvals. The associated risk, of course, depends on whether the users approve more than needed for a swap or do unlimited approvals which, however, are common use cases.

Protocol Configuration Assumptions

Fills are distinguished on the basis of their hashes. If two fills satisfy the protocol rules and fill the same deposit but have different hashes then the protocol refunds twice for one deposit. In particular, the concept of equivalent tokens introduces such a risk in case more than one equivalent token per chain is defined for a token. For example, USDC and USDC.e are two versions of the USDC token on Arbitrum. If both were defined as an equivalent token for USDC, the aforementioned scenario is possible.

 
 

Medium Severity

Deadline Buffer for Fills Is Not Always Respected

A newly added feature to the system is that the fill requests are subject to a deadline after which any fill attempt reverts. The essence of this feature is to limit the maximum necessary lookback for dataworkers and relayer instances.

For backwards compatibility reasons, the _deposit function will be present for a long time after the system upgrade introducing the new V3 interface takes place. However, note that the _deposit function sets each fill's deadline to an infinitely large number. As a consequence, even after the system's upgrade, new deposits will remain active practically forever, thereby breaking the assumption of the limited maximum necessary lookback.

Consider setting the default fill deadline equal to the standard deadline buffer in order to enable limited lookbacks.

Update: Resolved in pull request #14 at commit 71383bc. The Risk Labs team stated:

We decided not to implement the suggested fix because the existing integrators that use deposit() are likely not prepared to receive expired deposit refunds on the origin chain. Instead, we will enshrine MAX_UINT_256 as a “magic number” in the Across UMIP that is only possible to be set via the soon-to-be deprecated deposit() function.

This means that deposits sent from this function can never expire, and therefore the dataworker does not need to maintain a longer lookback to look out for these. Moreover, these deposits can always be slow-filled because outputToken is hardcoded to 0x0 by deposit(), meaning that these deposits will never lock funds.

Refund Leaf Execution Fails if One of the Addresses Is Blacklisted

If an address is blacklisted in the USDC token then both to and from transfers revert for this address. During refund leaf execution, tokens are pushed to the recipients. If one of the recipients is a blacklisted address, the whole execution fails and the blacklisted address as well as other addresses included in the leaf are not refunded. While such issues are inherent to the push pattern as opposed to the pull pattern, in this case, these issues and those similar to them can be remedied off-chain.

Consider adding a fallback option to the off-chain process that can separate problematic addresses to their own refund leaf so that they do not affect legitimate addresses.

Update: Acknowledged, will resolve. The Risk Labs team stated:

We will address this in the upcoming UMIP changes for Across V3. Relayer refund leaves that refund any user on the l2Token's blacklist should not be created.

Permit2OrderLib Is Not Fully Compliant With EIP-712

In the Permit2OrderLib contract, the witnessTypeString argument when calling the external permit2WitnessTransferFrom function does not fully follow the EIP-712 specification.

Specifically:

Consider fixing all the inconsistencies described above in order to fully comply with the EIP-712 specification.

Update: Resolved in pull request #15 at commit 446aae6.

Polygon_SpokePool-Specific Lock Can Be Bypassed

The Polygon_SpokePool contract has locks that prevent filling deposits with a message hook if a leaf is claimed within the same transaction. This is intended to prevent a situation in which the attacker executes a refund leaf and gets the control flow.

This lock, however, can be bypassed by executing the multicall function within which a relayer refund leaf is executed and a deposit is filled. The fill does not contain a message hook but during a native token transfer or a malicious outputToken transfer, it still gives the control flow to the attacker which defeats the purpose of the lock.

Consider adding a contract-wide lock to the executeV3RelayerRefundLeaf and executeRelayerRefundLeaf functions of Polygon_SpokePool so that each of these functions can only be executed if it is the only one in a transaction.

Update: Resolved in pull request #24 at commit d910641 and in pull request #29 at commit 8d6682b. The executeV3RelayerRefundLeaf function has been completely removed. Additionally, it is now prohibited to call executeRelayerRefundLeaf combined with other public function calls within a multicall transaction.

Low Severity

Unchecked Return Value

In the SwapAndBridge contract, the _swapAndBridge function attempts to pull the caller's tokens but does not check the return value of the transferFrom call.

Consider using the safeTransferFrom function of the SafeERC20 library in order to assert the call's success.

Update: Resolved in pull request #16 at commit 3a105a2.

Fillers Might Lose Their Collateral

The Permit2Depositor contract is meant to be used by third-party fillers and requires collateral from their side which is returned during the refund process. Because of this, they are also set as the exclusive relayer for the deposit. This way, they are protected from someone else filling the deposit and claiming the collateral.

However, liquidity providers can bypass the exclusivity restriction and have an incentive to do so by slow-filling the deposit thus leaving the collateral in the protocol. They can achieve this by requesting and executing a slow fill before the exclusivity deadline. This, of course, requires the filler to not to fill for at least the root bundle dispute period because slow fills can happen only after the root bundle dispute period has passed.

Consider either restricting the caller of the requested slow fill function to the depositor for the duration of the exclusivity period or guiding the third-party integrations to set the deposit lifespan to be less than the challenge period. The former approach gives more flexibility in terms of deposit lifespan but forces users to do on-chain calls if they want a slow fill for a deposit with an exclusive relayer. This might not be desirable given that the Permit2Depositor contract provides a gasless bridging experience to the user. The latter approach provides a better gasless experience but requires the third-party integration to be mindful of this issue.

Update: Resolved in pull request #17 at commit cc6b3cc. The Risk Labs team stated:

We ultimately decided to not only restrict when a slow fill request can be sent during the exclusivity window, but to block it altogether. We think that there is no reason for a depositor to request a slow fill when there is an exclusivity window set since they would have already made an off-chain agreement with an exclusive relayer to fill their deposit. If they had not, then they would set a 0-length exclusivity window. This implementation is the simplest to reason about without affecting projected UX.

Lack of Input Validation

There are some instances of input parameters within the SpokePool contract that are not sufficiently validated:

  • fillDeadline can be set in the past. In such a case, it is not possible to fill the deposit request.
  • exclusiveRelayer and exclusivityDeadline may not be consistent with each other. For example, it is possible that exclusiveRelayer is set to the 0x0 address while exclusivityDeadline is a non-zero value. This would create some confusion when filling the deposit request.
  • In the payable depositV3 function, msg.value should always be zero in case the inputToken is not the wrappedNativeToken.

Consider sufficiently validating all of the instances as described above.

Update: Resolved in pull request #18 at commit e1f2492. The Risk Labs team stated:

The new rule for exclusiveRelayer and exclusivityDeadline is that either both must be 0 or both must be non-zero and exclusivityDeadline >= currentTime.

Insufficient Access Controls

The Linea_SpokePool contract receives cross-chain messages from the HubPool via the claimMessage function of the Linea message service. The function sets the sender for the duration of the call. This might be problematic if the HubPool ever sends a cross-chain message to an attacker-controlled contract on Linea. This is because in this case, the attacker-controlled contract can make calls to Linea_SpokePool and bypass the access controls because the sender is set to the HubPool.

Consider checking both that sender is the HubPool and that the immediate caller is the Linea message service.

Update: Resolved in pull request #19 at commit e6bb797.

Low Level Call to External Exchange Contract With Arbitrary Calldata

The SwapAndBridge contract enables swapping an amount of tokens via a decentralized exchange service of preference and then bridge the swapped amount via Across. Both swap and bridging actions take place within a single transaction. To execute the swap, the SwapAndBridge contract performs a low level call to the designated exchange contract. For this call, the calldata parameters are arbitrarily given by the user. The only restriction is that the called function selector must be different than an ERC-20 token's transferFrom function. Performing a low level call with arbitrary data to an external contract should be avoided as it increases the possible attack surface.

Instead of blacklisting the suspicious calldata parameters, consider whitelisting the allowed ones. More specifically, consider whitelisting the specific swap function selectors of the external exchange in each SwapAndBridge contract.

Update: Resolved in pull request #20 at commit 2c68c2e.

Inconsistent And Misleading Events Emitted When Bridging To HubPool

A relayer refund leaf contains information about the refund amounts of a specific L2 token that need to be transferred to the relayers. In addition, it may also contain information about a non-zero amount of the L2 token that needs to be bridged back to the HubPool contract. Each L2 SpokePool contract implements the _bridgeTokensToHubPool function to handle the chain-specific operations for bridging to L1.

There is an inconsistency in the events emitted by the several L2 SpokePools upon bridging to HubPool. More specifically, the base SpokePool contract emits the TokensBridged event upon all relevant actions. Apart from this event, most of the L2 SpokePools emit one more event which typically contains 3 parts of information: the address of the L2 token, the receiver address (i.e., HubPool address), and the amount bridged.

However, the following inconsistencies and errors have been identified:

Consider addressing any faulty or inconsistent event information as listed above. Since most of the information contained in the more specific events is already covered by the event emitted by the base SpokePool contract, consider emitting a more specific event only when complementary information needs to be logged.

Update: Resolved in pull request #21 at commit d1aa772. The Risk Labs team stated:

We decided to remove all these chain-specific bridge events as they add no additional value to the TokensBridged event emitted in the parent contract and they are not currently used in any off-chain infrastructure.

Wrong Constructor Argument Passed

When constructing the UniversalSwapAndBridge contract, the address of the exchange contract is wrongly passed to SwapAndBridgeBase's constructor due to a typographical error. Specifically, instead of passing the _exchange parameter, exchange is passed, which is an uninitialized inherited variable. Since SwapAndBridgeBase does not validate _exchange against address(0), the faulty deployment would complete successfully.

Consider fixing the typographical error to avoid faulty deployments.

Update: Resolved in pull request #22 at commit 0a960f1.

Notes & Additional Information

Different Pragma Directives Are Used

Having fixed and the same pragma directives across file imports helps clearly identify the Solidity version with which the contracts will be compiled. However, the Permit2Depositor.sol file has the pragma directive pragma solidity ^0.8.0; and imports the Permit2OrderLib.sol file which has a different pragma directive.

Consider using the same fixed pragma version where possible.

Update: Resolved in pull request #23 at commit 5571b12. The Risk Labs team stated:

Changed pragma call in the following contracts: IPermit2, Permit2Order, Permit2OrderLib, and AddressLibUpgradeable.

Todo Comments in the Code

During development, having well-described TODO/Fixme comments will make the process of tracking and solving them easier. Without this information, these comments might age and important information for the security of the system might be forgotten by the time it is released to production. These comments should be tracked in the project's issue backlog and resolved before the system is deployed. There is a TODO comment at line 1221 in SpokePool.sol.

Consider removing all instances of TODO/Fixme comments and instead tracking them in the issues backlog. Alternatively, consider linking each inline TODO/Fixme to the corresponding issues backlog entry.

Update: Acknowledged, will resolve. The Risk Labs team stated:

We decided to leave in the TODO comments to remind ourselves to fix them in the future.

Inconsistent Use of Named Returns

Contract Arbitrum_Adapter contract has inconsistent usage of named returns in its functions. More specifically, only the _contractHasSufficientEthBalance function names its return variable.

To improve the readability of the contract, consider using the same return style in all of its functions.

Update: Resolved in pull request #23 at commit 5571b12.

Inconsistent Event Emission

In the Scroll_SpokePool contract, the initialize function directly sets the l2GatewayRouter and l2ScrollMessenger variables instead of calling their internal setters. As a consequence, the associated events will not be emitted. Similarly, the initialize function of the Polygon_SpokePool contract immediately sets the polygonTokenBridger and fxChild variables instead of calling the associated setter functions.

Consider calling the setter functions instead of directly setting the variables so that the associated events are emitted in a consistent manner.

Update: Resolved in pull request #23 at commit 5571b12. The Risk Labs team stated:

We refactored Scroll_SpokePool and Polygon_SpokePool to use the internal setter functions in the initializer.

Use of Magic Constants

The Arbitrum_SpokePool, OVM_SpokePool, and Polygon_SpokePool contracts use the magic constant 0 within their constructors to denote Circle's CCTP domain id for the Ethereum Mainnet. In addition, hardcoded 0 and address(0) values are used in the SpokePool contract which could be defined as constant variables.

Consider using a constant variable for each value of special meaning for better readability and clarity.

Update: Resolved in pull request #23 at commit 5571b12. The Risk Labs team stated:

We added the CCTP domain ID's as constants in CircleCCTPAdapter.sol. Magic values in Arbitrum_SpokePool, Polygon_SpokePool, andOVM_SpokePool have been replaced.

Incorrect or Misleading Docstrings

Several instances of incorrect or misleading docstrings have been identified throughout the codebase:

In SpokePool.sol:

  • At line 683, "plus" should be "less".
  • At line 528, "_deposit()" should be "deposit()".
  • At line 565, "_deposit()" should be "depositFor".

In Permit2Depositor.sol, at line 13, "AcrossV2" should be "AcrossV3".

In Permit2Order.sol, at line 59, "signature" could be more specifically described as "Permit2 signature".

Consider updating the misleading instances of docstrings for improved clarity and readability.

Update: Resolved in pull request #23 at commit 5571b12.

Unused Modifier

The onlyFromCrossDomainAccount modifier of the CrossDomainEnabled contract is never used within the entire codebase.

Consider removing the unused modifier for clarity and readability.

Update: Resolved in pull request #23 at commit 5571b12.

SwapAndBridge Contract Could Be Abstract

The SwapAndBridgeBase contract could be marked abstract since it only contains internal functions and is not intended to be directly deployed as a standalone contract.

Consider marking the SwapAndBridge contract abstract for clarity.

Update: Resolved in pull request #23 at commit 5571b12.

Naming Suggestion

The Polygon_SpokePool contract disallows relayer refunds and fills with a message hook to take place within a single transaction. This is achieved by setting and checking the lock variables. In essence, the _setFunctionLock function is responsible for locking a designated lock variable, while the _revertIfFunctionCalledAtomically function checks whether the lock variable is locked, and if so, reverts the execution.

Despite these two functions performing symmetric checks, their names do not represent this fact. As such, to improve the contract's overall clarity and readability, consider renaming _revertIfFunctionCalledAtomically to a more intuitive name (e.g., _revertIfFunctionLockSet).

Update: Resolved in pull request #24 at commit d910641. The Risk Labs team stated:

These hooks were completely removed by the fix for the M04 issue.

Constants Not Using UPPER_CASE Format

Throughout the codebase, there are constants that do not use the UPPER_CASE format.

According to the Solidity Style Guide, constants should be named with all capital letters with underscores separating words. For better readability, consider following this convention.

Update: Resolved in pull request #23 at commit 5571b12. The Risk Labs team stated:

We renamed variables in: BondToken, Ovm_SpokePool, PolygonTokenBridger, PolygonZkEVM_SpokePool, Arbitrum_Adapter, Base_Adapter, Boba_Adapter, Linea_Adapter, Optimism_Adapter, PolygonZkEVM_Adapter, Polygon_Adapter, Scroll_Adapter, and Permit2Depositor.

Lack of Security Contact

Providing a specific security contact (such as an email or ENS name) within a smart contract significantly simplifies the process for individuals to communicate if they identify a vulnerability in the code. This practice is quite beneficial as it permits the code owners to dictate the communication channel for vulnerability disclosure, eliminating the risk of miscommunication or failure to report due to a lack of knowledge on how to do so. In addition, if the contract incorporates third-party libraries and a bug surfaces in those, it becomes easier for the maintainers of those libraries to contact the appropriate person about the problem and provide mitigation instructions. Throughout the codebase, there are contracts that do not have a security contact.

Consider adding a NatSpec comment containing a security contact above the contract definitions. Using the @custom:security-contact convention is recommended as it has been adopted by the OpenZeppelin Wizard and the ethereum-lists.

Update: Resolved in pull request #23 at commit 5571b12. The Risk Labs team stated:

Security contact information has been added to SpokePool.sol

Variables Could Be constant

If a variable is only ever assigned a value when it is declared, then it could be declared as constant. Throughout the codebase, there are several variables that could be constant:

To better convey the intended use of variables and to potentially save gas, consider adding the constant keyword to variables that are only set when they are declared.

Update: Resolved in pull request #23 at commit 5571b12.

Unused Event

In SpokePool.sol, the RefundRequested event is unused.

To improve the overall clarity, intentionality, and readability of the codebase, consider removing the unused event.

Update: Resolved in pull request #23 at commit 5571b12. The Risk Labs team stated:

The event has been removed.

Unnecessary Casts

Throughout the codebase, there are multiple unnecessary casts:

To improve the overall clarity, intent, and readability of the codebase, consider removing unnecessary casts.

Update: Resolved in pull request #23 at commit 5571b12 and in pull request #30 at commit 2d90d5b.

Code Duplication

The IMessageService, ITokenBridge, and IUSDCBridge interfaces are defined in both the Linea_Adapter and the Linea_SpokePool contracts.

Consider including these interfaces in a separate file and importing them wherever needed in order to avoid duplicating the code which can be error-prone.

Update: Resolved in pull request #23 at commit 5571b12. The Risk Labs team stated:

They have been moved to the LineaInterfaces.sol contract.

Permit2Depositor Restricts Exclusive Relayer To msg.sender

The permit2Deposit function of the Permit2Depositor contract receives a signed order and makes a V3 deposit on behalf of the signer. The deposit made through the Permit2Depositor contract can only be filled by the designated exclusive relayer as the entire fill period is set exclusive. The exclusive relayer is always the caller of permit2Depositor.

If the Permit2 functionality gets supported on zkSync Era, the current version of Permit2Depositor would not function properly because of the different address derivation algorithm. Moreover, fillers may have different addresses on different chains so they might need more flexibility in specifying the exclusive relayer.

Consider adding the filler's address on the destination chain within the order's argument for flexibility but also to avoid faulty implementations on future upgrades that involve zkSync Era.

Update: Resolved in pull request #23 at commit 5571b12. The Risk Labs team stated:

The address destinationChainFillerAddress parameter has been added to permit2Deposit.

 

Recommendations

Codebase Can Benefit From Additional Testing

We recommend adding more unit and integration tests for the new components. In particular, tests for the Permit2Depositor and SwapAndBrdige contracts were not found. Tests can aid in communicating the intended behaviour and making the codebase more robust.

 

Conclusion

This audit mainly covered the code upgrade from V2 to V3 version of the protocol. Other changes audited include the additional features supported by the newly introduced peripheral contracts, the integration with the CCTP service for faster cross-chain USDC transfers, and the support of three new L2 chains.

The audited codebase largely involves integration and cooperation with external smart contracts (e.g., canonical bridges, exchange services, CCTP bridges, etc.) and great effort was made to investigate the possible attack vectors introduced by this fact.

The Risk Labs team provided us with extensive documentation about the version upgrade and newly introduced features, all the while being very responsive and engaging in discussions with the auditors.

 

Related Posts