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

Lack of Validation in setL1L2CollectionMapping Function Allows Creation of Invalid Address Mappings

lordofterra

Summary

The setL1L2CollectionMapping function in the Starklane contract does not perform any validation on the collectionL1 and collectionL2 addresses before setting the mapping. This lack of validation allow an attacker or a user to set arbitrary or invalid addresses as mappings, potentially leading to unexpected or insecure behavior in the contract.

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

Proof Of Concept

Coped POC

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;
​
import "forge-std/Test.sol";
import "../src/Bridge.sol";
import "../src/sn/Cairo.sol";
import "../src/token/ERC721Bridgeable.sol";
​
contract StarklaneTest is Test {
Starklane public starklane;
address public owner;
address public validL1Collection;
address public nonContractAddress;
uint256 public validL2Address;
​
function setUp() public {
owner = address(this);
validL1Collection = address(new ERC721Bridgeable());
nonContractAddress = address(0x1234);
validL2Address = uint256(uint160(0x1234567890123456789012345678901234567890));
​
starklane = new Starklane();
starklane.initialize(
abi.encode(
owner,
address(0x1234), // StarknetMessaging address
uint256(0x5678), // L2 address
uint256(0x9ABC) // L2 selector
)
);
}
​
function testInvalidL1L2Mapping() public {
// Test 1: Mapping non-contract address to L2
starklane.setL1L2CollectionMapping(nonContractAddress, Cairo.snaddressWrap(validL2Address), false);
​
// Test 2: Overwriting existing mapping without force flag
uint256 newL2Address = uint256(uint160(0x9876543210987654321098765432109876543210));
starklane.setL1L2CollectionMapping(validL1Collection, Cairo.snaddressWrap(newL2Address), false);
​
// Test 3: Overwriting existing mapping with force flag
uint256 anotherL2Address = uint256(uint160(0xabCDeF0123456789AbcdEf0123456789aBCDEF01));
starklane.setL1L2CollectionMapping(validL1Collection, Cairo.snaddressWrap(anotherL2Address), true);
​
// Instead, we'll check if the function executes without reverting, which it shouldn't for invalid inputs.
assertTrue(true, "All setL1L2CollectionMapping calls should succeed without proper validation");
}
}

Vulnerability Details

The test results indicate that the setL1L2CollectionMapping function allows the setting of arbitrary or invalid L1 and L2 addresses without any validation. Below is a breakdown of the relevant evidence from the test execution:

1: Setting an Invalid L1 Collection:

  • L1 Collection: 0x0000000000000000000000000000000000001234

  • L2 Collection: 103929005307130220006098923584552504982110632080

  • Active: false

  • Event Emitted: L1L2CollectionMappingUpdated was emitted, indicating that the mapping was set despite the invalid nature of the collectionL1 address.

├─ [48975] Starklane::setL1L2CollectionMapping(0x0000000000000000000000000000000000001234, 103929005307130220006098923584552504982110632080 [1.039e47], false)
│ ├─ emit L1L2CollectionMappingUpdated(colllectionL1: 0x0000000000000000000000000000000000001234, collectionL2: 103929005307130220006098923584552504982110632080 [1.039e47])
│ └─ ← [Stop]

2: Setting a Valid L1 Collection with Arbitrary L2 Address:

  • L1 Collection: ERC721Bridgeable

  • L2 Collection: 870405419566846112171829716905735280232177611280

  • Active: false

  • Event Emitted: L1L2CollectionMappingUpdated was emitted without validating the correctness of the L2 address.

├─ [46975] Starklane::setL1L2CollectionMapping(ERC721Bridgeable: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 870405419566846112171829716905735280232177611280 [8.704e47], false)
│ ├─ emit L1L2CollectionMappingUpdated(colllectionL1: ERC721Bridgeable: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], collectionL2: 870405419566846112171829716905735280232177611280 [8.704e47])
│ └─ ← [Stop]

3: Setting a Valid L1 Collection with Another Arbitrary L2 Address:

  • L1 Collection: ERC721Bridgeable

  • L2 Collection: 980829894800078742339098726298021775428853559041

  • Active: true

  • Event Emitted: L1L2CollectionMappingUpdated was emitted again, confirming the lack of validation.

├─ [24900] Starklane::setL1L2CollectionMapping(ERC721Bridgeable: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], 980829894800078742339098726298021775428853559041 [9.808e47], true)
│ ├─ emit L1L2CollectionMappingUpdated(colllectionL1: ERC721Bridgeable: [0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f], collectionL2: 980829894800078742339098726298021775428853559041 [9.808e47])
│ └─ ← [Stop]

Impact

1: Invalid mappings could break functionality during operations that rely on the correctness of these mappings, such as token withdrawals or transfers.

2: An attacker could exploit this vulnerability to create mappings that could lead to unauthorized access or manipulation of tokens or other critical operations within the contract.

3: Mappings to non-existent or incorrect addresses could cause disruptions in the operation of the contract, potentially leading to loss of funds or other critical errors.

Tools Used

Manaul review

Recommendations

1: Introduce validation checks within the setL1L2CollectionMapping function to ensure that the collectionL1 and collectionL2 addresses are valid, correctly formatted, and point to actual contracts or addresses that meet specific criteria.

2: Use require() statements to enforce the validity of the addresses. This would prevent the function from proceeding if the provided addresses do not meet the necessary conditions.

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.