NFTBridge
60,000 USDC
View results
Submission Details
Severity: high
Invalid

L2 to L1 Address Mapping Overwrite in `CollectionManager.sol`

Summary

The _deployERC721Bridgeable function in the CollectionManager.sol contract allows the L1 address associated with an L2 collection to be overwritten unintentionally when multiple users bridge tokens from different L1 addresses to the same L2 collection. This can lead to incorrect token associations and permanent loss of user funds.

Vulnerability Details

The vulnerability lies in the _deployERC721Bridgeable function, which is responsible for deploying a new ERC721Bridgeable contract on L1 when a native L2 collection is bridged to L1 for the first time. The function updates the _l2ToL1Addresses mapping to associate the L2 collection address with the newly deployed L1 contract address.

However, the function overwrites any existing L1 address associated with the L2 collection without checking if a mapping already exists. This means that if multiple users bridge tokens from different L1 addresses to the same L2 collection, each new bridge operation will overwrite the previous L1 address in the _l2ToL1Addresses mapping.

As a result, the bridge will only remember the most recent L1 address, losing the association with all previous L1 addresses. When users attempt to withdraw their tokens back to L1, the bridge will use the incorrect L1 address from the overwritten mapping, causing tokens to be sent to the wrong address.

┌────────────┐
│ L1 Bridge │
└────────────┘
▲ │
│ │ _l2ToL1Addresses Mapping
│ │ (Incorrectly Overwritten)
│ ▼
┌────────────┐ ┌────────────┐
│ 0xAlice │ │ 0xBob │
└────────────┘ └────────────┘
▲ ▲
│ │
│ │ Owns Kitty #456
│ Owns Kitty #123
│ │
│ Deposit │ Deposit
▼ ▼
┌────────────┐
│ Escrow │
└────────────┘
▲ │
│ │ Deposit Requests
│ ▼
┌────────────┐
│ L2 Bridge │
└────────────┘
▲ │
│ │ CryptoKitties Collection (L2)
│ ▼
┌────────────┐ ┌────────────┐
│ Kitty #123 │ │ Kitty #456
└────────────┘ └────────────┘

Impact

The impact of this vulnerability is severe, as it can lead to a permanent loss of user funds. If a user bridges tokens to an L2 collection and then another user bridges tokens from a different L1 address to the same L2 collection, the second user's L1 address will overwrite the first user's address in the _l2ToL1Addresses mapping.

When the first user tries to withdraw their tokens back to L1, the bridge will send the tokens to the second user's L1 address instead of the original address. The first user will lose access to their tokens, and there is no way for them to recover the tokens sent to the incorrect address.

This vulnerability can affect multiple users, with each new user potentially overwriting the L1 addresses of all previous users for a given L2 collection. The more users interact with the bridge, the higher the risk of funds being lost.

Tools Used

  • Manual code review

  • Solidity compiler

  • Foundry for code analysis and testing

Recommendations

To mitigate this vulnerability, it is recommended to modify the _deployERC721Bridgeable function to include a check for existing L1 address mappings before updating the _l2ToL1Addresses mapping. The following code change should be implemented:

function _deployERC721Bridgeable(
// ...
)
internal
returns (address)
{
// ...
// Check if a mapping already exists for collectionL2
if (_l2ToL1Addresses[collectionL2] != address(0)) {
revert("L1 address already exists for this L2 collection");
}
_l1ToL2Addresses[proxy] = collectionL2;
_l2ToL1Addresses[collectionL2] = proxy;
// ...
}

By adding this check, the function will revert if an L1 address is already associated with the given L2 collection, preventing the overwrite of existing mappings. This ensures that the original L1 address is preserved, and users can always withdraw their tokens to the correct address.

It is crucial to thoroughly test the implementation of this fix and validate that it effectively prevents the vulnerability in all possible scenarios. Additionally, it is recommended to conduct a comprehensive audit of the entire bridge codebase to identify and address any similar vulnerabilities that may exist in other parts of the system.

By implementing the recommended fix and conducting a thorough security audit, the bridge can ensure the security and reliability of its token associations, protecting users' funds and maintaining the integrity of the system.

Updates

Lead Judging Commences

n0kto Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.