ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

CollectionManager.sol :: When force is set to true in _setL1L2AddressMapping(), does not verify if collectionL2 is a valid felt252, potentially causing issues in L2 and preventing the message from being delivered.

ivanfitro

Summary

_setL1L2AddressMapping() is used to link addresses in L1 and L2. However, when the force flag is true, the mappings are set without validating if the collectionL2 is a valid felt252. This can lead to issues if the value exceeds the felt prime number, causing potential problems in L2 and preventing the message from being delivered.

Vulnerability Details

_setL1L2AddressMapping() is implemented as follows.

function _setL1L2AddressMapping(
address collectionL1,
snaddress collectionL2,
bool force
)
internal
{
if (((snaddress.unwrap(_l1ToL2Addresses[collectionL1]) == 0) && (_l2ToL1Addresses[collectionL2] == address(0)))
|| (force == true)) {
_l1ToL2Addresses[collectionL1] = collectionL2;
_l2ToL1Addresses[collectionL2] = collectionL1;
} else {
revert CollectionMappingAlreadySet();
}
}

When force == true, the mappings are set directly without verifying that collectionL2 is not higher than the felt prime number, which can cause issues on L2 and prevent the message from arriving. The mapping _l2ToL1Addresses is utilized in depositTokens().

function depositTokens(
uint256 salt,
address collectionL1,
snaddress ownerL2,
uint256[] calldata ids,
bool useAutoBurn
)
external
payable
{
///...
​
req.header = Protocol.requestHeaderV1(ctype, useAutoBurn, false);
req.hash = Protocol.requestHash(salt, collectionL1, ownerL2, ids);
// TODO: store request hash in storage to avoid replay attack.
// or can it be safe to use block timestamp? Not sure as
// several tx may have the exact same block.
req.collectionL1 = collectionL1;
@> req.collectionL2 = _l1ToL2Addresses[collectionL1];
​
req.ownerL1 = msg.sender;
req.ownerL2 = ownerL2;
​
///...
}

If the L2 address is not correctly set, the message will fail to arrive.

Impact

The message may never arrive.

Tools Used

Manual review.

Recommendations

To solve the problem, ensure that collectionL2 is a valid felt252.

function _setL1L2AddressMapping(
address collectionL1,
snaddress collectionL2,
bool force
)
internal
{
​
+ if (!Cairo.isFelt252(snaddress.unwrap(collectionL2))) {
+ revert CairoWrapError();
+ }
​
if (((snaddress.unwrap(_l1ToL2Addresses[collectionL1]) == 0) && (_l2ToL1Addresses[collectionL2] == address(0)))
|| (force == true)) {
​
_l1ToL2Addresses[collectionL1] = collectionL2;
_l2ToL1Addresses[collectionL2] = collectionL1;
} else {
revert CollectionMappingAlreadySet();
}
}
​
Updates

Lead Judging Commences

n0kto Lead Judge 10 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.