Hawk High
First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

[H-2] Storage Layout Collision During Contract Upgrade

nymuelis

Severity

Critical

Impact

The LevelOne and LevelTwo contracts have incompatible storage layouts, which will cause severe data corruption during upgrade. This will result in:

  • Loss of critical school fee data

  • Misalignment of all state variables (values will be assigned to incorrect variables)

  • Complete loss of review tracking functionality (mappings missing in LevelTwo)

  • Potential fund misallocation during graduation

  • Permanent corruption of contract state with no recovery method

These issues compromise the entire functionality of the upgraded contract and will lead to immediate financial loss as funds are transferred to incorrect addresses or become permanently locked.

Description

When upgrading from LevelOne to LevelTwo using the UUPS pattern, the storage layout must remain compatible to preserve data integrity. However, several critical variables are missing or reordered in LevelTwo:

In LevelOne, the storage variables begin with:

address principal;
bool inSession;
uint256 schoolFees; // <-- Missing in LevelTwo
uint256 public immutable reviewTime = 1 weeks; // Immutable (doesn't affect storage)
uint256 public sessionEnd;
uint256 public bursary;
// ...
mapping(address => uint256) private reviewCount; // <-- Missing in LevelTwo
mapping(address => uint256) private lastReviewTime; // <-- Missing in LevelTwo

In LevelTwo, the variables are:

address principal;
bool inSession;
uint256 public sessionEnd; // <-- Now in the position where schoolFees was
uint256 public bursary; // <-- Shifted up
uint256 public cutOffScore; // <-- Shifted up
// ...

The missing schoolFees variable creates a ripple effect where all subsequent storage slots become misaligned. After upgrade, the following critical data corruption will occur:

  1. sessionEnd in LevelTwo will contain the value of schoolFees from LevelOne

  2. bursary will contain the value of sessionEnd, meaning the financial accounting for the system is completely broken

  3. cutOffScore will contain the value of bursary, meaning student graduation requirements become whatever the bursary amount was

  4. All teacher and student data will be shifted, causing cross-contamination between mappings

  5. The 60% of bursary funds meant to be carried forward will be accessible in the wrong storage slot

Since this storage collision affects financial data and access controls, it represents a catastrophic failure mode for the entire system.

Recommended Mitigation

To preserve storage compatibility:

  1. Ensure LevelTwo declares all variables in the exact same order as LevelOne, even if they're unused:

address principal;
bool inSession;
uint256 schoolFees; // Keep this even if unused in LevelTwo
uint256 public immutable reviewTime = 1 weeks;
uint256 public sessionEnd;
// ...etc.

  1. Add back the missing mappings:

mapping(address => uint256) private reviewCount;
mapping(address => uint256) private lastReviewTime;

  1. Consider using OpenZeppelin's StorageSlot library or the more recent ERC-7201 Namespaced Storage Layout pattern for better storage management.

  2. Add extensive tests to verify storage compatibility between contract versions.

References

A PoC isn't necessary as this is a deterministic storage layout issue that can be verified through code inspection alone. The corruption occurs at upgrade time and is irreversible once executed.

Updates

Lead Judging Commences

yeahchibyke Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

storage collision

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.