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

Inconsistent Validation of Starknet Addresses in L1

Summary

The L1 bridge contract does not consistently validate Starknet addresses to ensure they are within the valid range for the Starknet network. While some parts of the code perform this check, it is not uniformly applied across all functions handling Starknet addresses, potentially leading to security vulnerabilities.

This is similar to "[M-01] Unchecked parameter" on the Clan audit report.

Vulnerability Details

The contract uses a custom snaddress type to represent Starknet addresses, which is an alias for uint256. Some functions in the contract correctly validate these addresses using the Cairo.isFelt252() function:

if (!Cairo.isFelt252(snaddress.unwrap(ownerL2))) {
revert CairoWrapError();
}

Impact

https://github.com/Cyfrin/2024-07-ark-project/blob/8f4f71d8b6487c316334a7e427f888cda01c8cff/apps/blockchain/ethereum/src/Bridge.sol#L373

Will cause permenent loss of tokens when L2 address is invalid

Tools Used

Manual review

Recommendations

Verify if the address in valid on setL1L2CollectionMapping()

function setL1L2CollectionMapping(
address collectionL1,
snaddress collectionL2,
bool force
) external onlyOwner {
if (!Cairo.isFelt252(snaddress.unwrap(collectionL2))) {
revert CairoWrapError();
}
_setL1L2AddressMapping(collectionL1, collectionL2, force);
emit L1L2CollectionMappingUpdated(collectionL1, snaddress.unwrap(collectionL2));
}
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.