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

Storage Layout Mismatch (State Variable Order Inconsistency + Storage Collision)

jufel

Description: The two implementations, 'LevelOne' and 'LevelTwo', declare their state variables in different orders. When you perform a UUPS upgrade, the proxy's storage is interpreted by the new implementation according to its own layout. Mismatched layouts cause storage collisions—slots written by one implementation map to the wrong variables in the next.

Impact:

  • Corrupted state: Data intended for 'bursary' in LevelOne end up in 'cutOffScore' in LevelTwo, leading to wildly incorrect behavior.

  • Security bypasses: Critical invariants break, and funds or upgrades can be executed under the wrong conditions.

  • Fund loss or lock: Misinterpreted 'bursary' values can result in under- or over-payment, or leave residual balances stranded.

Proof of Concept:

LevelOne storage:

Name Type Slot Offset Bytes
principal address 0 0 20
inSession bool 0 20 1
schoolFees uint256 1 0 32
sessionEnd uint256 2 0 32
bursary uint256 3 0 32
cutOffScore uint256 4 0 32
isTeacher mapping(address => bool) 5 0 32
isStudent mapping(address => bool) 6 0 32
studentScore mapping(address => uint256) 7 0 32
reviewCount mapping(address => uint256) 8 0 32
lastReviewTime mapping(address => uint256) 9 0 32
listOfStudents address[] 10 0 32
listOfTeachers address[] 11 0 32
usdc contract IERC20 12 0 20

LevelTwo storage:

Name Type Slot Offset Bytes
principal address 0 0 20
inSession bool 0 20 1
sessionEnd uint256 1 0 32
bursary uint256 2 0 32
cutOffScore uint256 3 0 32
isTeacher mapping(address => bool) 4 0 32
isStudent mapping(address => bool) 5 0 32
studentScore mapping(address => uint256) 6 0 32
listOfStudents address[] 7 0 32
listOfTeachers address[] 8 0 32
usdc contract IERC20 9 0 20

Note: this PoC assumes that the 'Missing UUPS Inheritance in LevelTwo' issue has already been fixed, so that graduateAndUpgrade gets as far as splitting by totalTeachers instead of reverting earlier.

+ import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
​
- contract LevelTwo is Initializable {
+ contract LevelTwo is Initializable, UUPSUpgradeable {
+ constructor() { _disableInitializers(); }
+ function _authorizeUpgrade(address newImplementation) internal override onlyPrincipal {}

After this fix include the following test in the LevelOneAndGraduateTest.t.sol file:

function testBursaryAppearsAsSessionEndInV2() public schoolInSession {
uint256 bursaryV1 = levelOneProxy.bursary();
​
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
​
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, "");
​
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
​
uint256 bursaryV2 = levelTwoProxy.bursary();
​
vm.expectRevert();
assertEq(bursaryV1, bursaryV2, "Bursary should be equal to bursary in V2");
}

Recommended Mitigation:
Adopt a stable storage layout pattern across all versions:

  1. Keep variable order identical in both implementations.

  2. Insert new state variables only at the end of the layout.

  3. Reserve gaps for future growth, e.g.:

// after existing variables
uint256[50] private __gap;

This ensures that every storage slot’s meaning remains consistent through each upgrade, preventing collisions and preserving all invariants.

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.