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

Validation missing for `collectionL2` felt value in `setL1L2CollectionMapping` function

Github

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/Bridge.sol#L368-L375

Summary

The setL1L2CollectionMapping function in the Bridge.sol contract does not check if the collectionL2 parameter, of type snaddress, is a valid felt value. This could potentially lead to issues if an invalid value is entered, even though the function is restricted to the contract owner. Similar issues in the previous audit is marked mediumbut i am submitting this one as lowbecause the function is restricted so likelihood is low but the impact can be high.

Impact

Even though the function is restricted to the admin (trusted role), there is a risk that an invalid felt value could be entered for collectionL2, leading to potential operational issues or failures in the bridge functionality.

Proof of concept

The current implementation of the function does not validate that collectionL2 is a valid felt value:

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

Without validation, if collectionL2 is not within the valid range for felt values, it can cause issues when interacting with StarkNet.

Recommendation

Add a validation check to ensure collectionL2 is within the valid range for felt values.

function setL1L2CollectionMapping(
address collectionL1,
snaddress collectionL2,
bool force
) external onlyOwner {
require(Cairo.isFelt252(snaddress.unwrap(collectionL2)), "Invalid felt252 value for collectionL2");
_setL1L2AddressMapping(collectionL1, collectionL2, force);
emit L1L2CollectionMappingUpdated(collectionL1, snaddress.unwrap(collectionL2));
}

By adding this validation, the function will ensure that collectionL2 is always a valid felt value.

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.