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

Changes to the `L1 <-> L2` mapping causes loss of NFTs on L1

Summary

If the admin changes the L1 <-> L2 mapping on L1, not-yet withdrawn NFTs cannot be withdrawn.

Vulnerability Details

The mapping for L1 to L2 collections on L1 dictates to which collection a L2 -> L1 bridged NFT belongs. If a request's destination collection is non-zero and does not match the one in the mapping, this NFT can not be withdrawn because _verifyRequestAddresses called by withdrawTokens will revert:

function _verifyRequestAddresses(
address collectionL1Req,
snaddress collectionL2Req
)
internal
view
returns (address)
{
address l1Req = collectionL1Req;
uint256 l2Req = snaddress.unwrap(collectionL2Req);
address l1Mapping = _l2ToL1Addresses[collectionL2Req];
uint256 l2Mapping = snaddress.unwrap(_l1ToL2Addresses[l1Req]);
// [...]
// L2 address is present, and L1 address too.
if (l2Req > 0 && l1Req > address(0)) {
if (l1Mapping != l1Req) {
revert InvalidCollectionL1Address(); // [1] <-----
} else if (l2Mapping != l2Req) {
revert InvalidCollectionL2Address(); // [2] <-----
} else {
// All addresses match, we don't need to deploy anything.
return l1Mapping;
}
}
revert ErrorVerifyingAddressMapping();
}

As we see at [1] and [2], if there is a mismatch, the transaction gets reverted. Now this is a problem because the mapping can change and if this transaction reverts, the NFT is stuck as it is impossible to cancel a L2 -> L1 bridging.

Now let's assume the following scenario:

  • A collection is deployed on L1 and L2 -> mappings are set

  • User bridges NFT from L2 to L1 -> destination collection is non-zero

  • User does not withdraw the NFT yet and leaves it in the bridge

  • An admin changes the mapping on L1 (for whatever reason)

  • User can not withdraw their NFT anymore because any calls to withdrawTokens revert

Impact

The impact of this is loss of NFTs as described above.

Tools Used

Manual review

Recommended Mitigation

Consider one of the following:

  • Not allowing mapping changes if NFTs of that collection are held in the bridge

  • Allowing withdrawals even if the mapping does not match the request

Updates

Lead Judging Commences

n0kto Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational / Gas

Please, do not suppose impacts, think about the real impact of the bug and check the CodeHawks documentation to confirm: https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity A PoC always helps to understand the real impact possible.

Support

FAQs

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