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

[H-2] Storage‑Layout Collision Risk on Future Upgrade

cosminmarian53

Summary

The LevelOne → LevelTwo UUPS upgradeable contracts suffer from a storage‐layout collision: after slot 0, every variable in LevelTwo shifts and reinterprets the proxy’s existing storage, corrupting state. This latent bug lies dormant until the proxy is correctly upgraded, at which point schoolFees (slot 1) becomes sessionEnd, sessionEnd (slot 2) becomes bursary, and so on, silently overwriting balances, scores, and mappings. If exploited or simply triggered by a future upgrade, this will break business logic and could cause substantial token loss. Mitigation requires aligning storage (inheritance or gaps) and adding automated CI checks.

Vulnerability Details

Storage Layout Comparison

Slot LevelOne LevelTwo
0 address principalbool bool inSession address principalbool bool inSession
1 uint256 schoolFees uint256 sessionEnd
2 uint256 sessionEnd uint256 bursary
3 uint256 bursary uint256 cutOffScore
4 uint256 cutOffScore mapping(address=>bool) isTeacher
… … …

Because LevelTwo neither inherits LevelOne nor reserves unused slots, every subsequent variable is repurposed on upgrade, leading to storage collision

Impact


Critical fields like bursary (accumulated USDC) may map to a student’s cut‑off score or teacher wage slots, enabling arbitrary transfers or lost funds.

Student scores, session flags, and teacher/ student rosters vanish or invert, halting core school operations.

Tools Used

  • Foundry

  • Manual Review

Proof of Concept

How It Happens

  1. Delegatecall Semantics: A proxy’s delegatecall means the implementation reads/writes the proxy’s storage.

  2. Mismatched Definitions: Changing order, removing, or adding fields without reserving gaps shifts all downstream slots.

  3. Silent Corruption: No compiler or runtime error—existing state is simply misinterpreted under the new layout.

    If the upgrade to LevelTwo issue would be fixed, then the following test holds true:

function test_storage_collision() public {
// 1. Read slot 1 (schoolFees) before upgrade
bytes32 rawBefore = vm.load(proxyAddress, bytes32(uint256(1)));
uint256 feesBefore = uint256(rawBefore);
assertEq(feesBefore, schoolFees);
​
// 2. Perform the UUPS upgrade to LevelTwo
levelTwoImplementation = new LevelTwo();
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(address(levelTwoImplementation), data);
​
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
​
// 3. Read slot 1 again (now interpreted as sessionEnd)
bytes32 rawAfter = vm.load(address(levelTwoProxy), bytes32(uint256(1)));
uint256 sessionEndSlot = uint256(rawAfter);
// It must still equal the same schoolFees value => collision
assertEq(sessionEndSlot, schoolFees);
}

Recommendations

  1. Either have LevelTwo inherit LevelOne so state variables preserve exact slot positions, like this:

contract LevelTwo is LevelOne { … }

  1. Or use blank storage variables in order to align storage state.


Updates

Lead Judging Commences

yeahchibyke Lead Judge 3 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.