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

[M-01] Risk of Storage Collision Due to State Variables in `Starklane` Contract

4rdiii

[M-01] Risk of Storage Collision Due to State Variables in Starklane Contract

Summary

The Starklane contract is UUPS-upgradeable and ownable , also inheriting state variables from multiple contracts. The use of boolean state variables such as _enabled and _whiteListEnabled introduces a risk of storage collision in future versions if the storage layout changes. Instead of using these booleans directly, it is advisable to use implementations like OpenZeppelin's PausableUpgradeable, which use unique storage locations for these variables to mitigate this risk.

Vulnerability Details

The current implementation of Starklane uses direct boolean state variables to manage the pause and whitelist functionalities. This approach is prone to storage collisions in future contract upgrades due to the complex inheritance and upgradeable nature of the contract. OpenZeppelin's PausableUpgradeable contract offers a more robust solution by assigning unique storage slots, reducing the risk of such collisions.

Given the potential for significant issues caused by storage layout changes, it seems more likely that this is a bug rather than an intentional design choice. The need for reliable pausability and whitelist functionalities suggests that a more resilient implementation is necessary.

contract Starklane is
IStarklaneEvent,
UUPSOwnableProxied,
StarklaneState,
StarklaneEscrow,
StarklaneMessaging,
CollectionManager
{
// Mapping (collectionAddress => bool)
mapping(address => bool) _whiteList;
address[] _collections;
@> bool _enabled;
@> bool _whiteListEnabled;
.
.
.
}

Impact

In future updates, the contract may experience storage collisions, potentially disrupting the pausability and whitelist functionalities. This could lead to serious security vulnerabilities, such as the inability to pause the contract during an emergency or failure of the whitelist mechanism, potentially allowing unauthorized access. The integrity and security of the contract could be compromised, leading to financial losses and reputational damage.
The risk can be demonstrated by considering the following scenario:

  1. Deploy the Starklane contract and set initial values for _enabled and _whiteListEnabled.

  2. Upgrade the contract to a new version that introduces new state variables, changing the storage layout.

  3. Observe that the storage slots for _enabled and _whiteListEnabled may overlap with the new variables, causing unintended behavior.

Tools Used

Manual Review,

Recommendations

To prevent storage collisions in future upgrades, it is recommended to use OpenZeppelin's PausableUpgradeable contract for pausability logic and a similar implementation for whitelist logic. OpenZeppelin's implementation ensures unique storage slot assignments, reducing collision risks. Here’s an example of how to update the contract, the same can be done for whitelist and also the functions which are effected by _enabled should be changed but its ignored here for simplicity.

import "@openzeppelin/contracts-upgradeable/security/PausableUpgradeable.sol";
​
contract Starklane is
IStarklaneEvent,
UUPSOwnableProxied,
StarklaneState,
StarklaneEscrow,
StarklaneMessaging,
CollectionManager,
+ PausableUpgradeable
{
// Mapping (collectionAddress => bool)
mapping(address => bool) _whiteList;
address[] _collections;
​
- bool _enabled;
bool _whiteListEnabled;
​
+ function initialize() initializer public {
+ __Pausable_init();
+ // other initializations
+ }
+ function pause() public onlyOwner {
+ _pause();
+ }
+ function unpause() public onlyOwner {
+ _unpause();
+ }
}
​
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.