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

Bridge.sol :: setL1L2CollectionMapping() does not whitelist collectionL1.

ivanfitro

Summary

setL1L2CollectionMapping() allows the owner to set the _l1ToL2Addresses and _l2ToL1Addresses mappings, linking collectionL1 to collectionL2. However, this function does not whitelist collectionL1 in the _whiteList mapping, preventing users from interacting with it.

Vulnerability Details

setL1L2CollectionMapping() is implemented as follows.

function setL1L2CollectionMapping(
address collectionL1,
snaddress collectionL2,
bool force
) external onlyOwner {
_setL1L2AddressMapping(collectionL1, collectionL2, force);
emit L1L2CollectionMappingUpdated(collectionL1, snaddress.unwrap(collectionL2));
}

The internal function _setL1L2AddressMapping() is invoked.

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();
}
}

While the mappings are established, the collectionL1 is not added to the whitelist. In contrast, new collections are whitelisted upon deployment, as demonstrated in the withdrawTokens(), where the mappings are also set.

function withdrawTokens(
uint256[] calldata request
)
external
payable
returns (address)
{
///...
​
if (collectionL1 == address(0x0)) {
if (ctype == CollectionType.ERC721) {
collectionL1 = _deployERC721Bridgeable(
req.name,
req.symbol,
req.collectionL2,
req.hash
);
// update whitelist if needed
@> _whiteListCollection(collectionL1, true);
} else {
revert NotSupportedYetError();
}
​
///...

When a new collection is deployed, it is whitelisted; however, this is not done in the setL1L2CollectionMapping(), leading to inconsistency. Consequently, the mappings are established, but users cannot bridge the collection because it is not whitelisted.

The solution involves manually calling whiteList(), but this creates an inconsistency, as the code behaves differently in withdrawTokens() compared to setL1L2CollectionMapping().

Impact

Users are unable to bridge using the new collection and also causes inconsistencies in the code.

Tools Used

Manual review.

Recommendations

To resolve the issue, whitelist collectionL1 in the setL1L2CollectionMapping().

function setL1L2CollectionMapping(
address collectionL1,
snaddress collectionL2,
bool force
) external onlyOwner {
_setL1L2AddressMapping(collectionL1, collectionL2, force);
+ _whiteListCollection(collectionL1, true);
emit L1L2CollectionMappingUpdated(collectionL1, snaddress.unwrap(collectionL2));
}
Updates

Lead Judging Commences

n0kto Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice
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.