ArkProject: NFT Bridge

Summary

The Starklane contract, designed to be an upgradable contract, inherits from multiple contracts (UUPSOwnableProxied, StarklaneState, StarklaneEscrow, StarklaneMessaging, and CollectionManager). Each of these contracts defines its own set of storage variables, but none of them reserve storage gaps for future upgrades. This makes it impossible to safely add new variables to any of the inherited contracts without causing storage collisions. The Starklane contract itself is the only safe place to add new variables, significantly limiting the flexibility of future upgrades and increasing the risk of storage collision.

Vulnerability Details

Inheritance Structure and Storage Collision

The Starklane contract's inheritance from multiple contracts creates a complex storage layout where each contract's variables are packed into storage slots based on the inheritance order. Because Solidity assigns storage slots sequentially based on the order in which contracts are inherited, adding new variables to any of the inherited contracts risks overlapping with storage slots already in use by other contracts.

For example, consider the following storage layout based on the inheritance order:

• UUPSOwnableProxied:

  • _initializedImpls (mapping) occupies storage slots based on its size and position.

• StarklaneState:

  • _starknetCoreAddress, _starklaneL2Address, and _starklaneL2Selector (address and felt252) occupy the next available slots.

• StarklaneEscrow:

  • _escrow (mapping) will occupy storage slots sequentially based on the inherited storage from the previous contracts.

• StarklaneMessaging:

  • _autoWithdrawn (mapping) continues to fill storage slots based on its size.

• CollectionManager:

  • _l2ToL1Addresses and _l1ToL2Addresses (mappings) occupy slots based on the inherited storage layout.

Proof Of Concept:
Assume the following storage assignments:

• UUPSOwnableProxied._initializedImpls: Slot 0

• StarklaneState._starknetCoreAddress: Slot 1

• StarklaneEscrow._escrow: Slots 2+

• StarklaneMessaging._autoWithdrawn: Slots 3+

• CollectionManager._l2ToL1Addresses: Slots 4+

If you add a new variable (e.g., uint256 newVariable) to StarklaneEscrow, it will likely overwrite the _autoWithdrawn mapping in StarklaneMessaging because they are contiguous in memory. This leads to a storage collision, causing unexpected behavior and data corruption.

Risk of Adding Variables to Inherited Contracts

Adding new variables to any inherited contract (except Starklane itself) will almost certainly lead to storage collisions. Solidity assigns storage slots based on inheritance order, so even appending variables to the end of each contract can result in overlap with slots assigned to other contracts in the inheritance chain.

Proof of Concept:

• Adding uint256 newVariable to StarklaneState could overwrite data in StarklaneEscrow , StarklaneMessagingand CollectionManager.

• Adding new variables to UUPSOwnableProxied will overwrite data in StarklaneState, StarklaneEscrow, or any subsequent contract in the inheritance chain.

Why Starklane Is the Only Safe Place to Add Variables

Because Starklane is the final contract in the inheritance chain, it occupies the last set of storage slots. As a result, adding new variables to Starklane itself is safe, as they will be placed after all other storage variables. However, this limits the ability to upgrade or modify any other part of the system.Impact.

Impact

Storage Collision

The risk of storage collision is exceptionally high. Any attempt to add new variables to any contract other than Starklane will almost certainly result in overlapping storage slots, causing:

• Data Corruption: Storage collisions will corrupt existing data, leading to incorrect contract behavior.

• Loss of Functionality: Functions relying on specific storage variables may fail or behave unpredictably.

• Impossible Upgrades: The current storage layout makes it nearly impossible to safely upgrade or extend functionality in any contract except Starklane.

Inability to Add New Variables

Due to the inheritance structure and storage layout, adding new variables to any contract other than Starklane will likely cause immediate storage collisions. This effectively locks the contract's storage layout, making future upgrades extremely risky and limiting the contract's extendability.

Tools Used

• Manual Code Review: Analyzed the contract's inheritance structure and storage layout.

• Foundry: Used this tool to simulate upgrades and identify potential storage collisions.

Recommendations

Use Storage Gaps

To prevent storage collisions, reserve storage slots in each contract to allow for future expansion without risking overlap. This is done by adding a storage gap:

This reserves 50 slots, which can be safely used in future upgrades.

Add the following storage gap to each contract:

Use Specific Storage Slot Assignments

Instead of relying on the default storage slot assignment, explicitly assign storage slots to variables. This ensures that new variables do not accidentally overwrite existing data.
Assign a specific storage slot to each variable: