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

Storage Layout Inconsistency Between LevelOne and LevelTwo Implementations

dynamo

Summary

The LevelTwo implementation does not maintain consistent storage layout with LevelOne, creating a dangerous mismatch between the contract's code and its storage. After upgrading, functions from LevelOne remain accessible through the proxy and operate on storage variables not declared in LevelTwo, risking silent data corruption.

Vulnerability Details

// In LevelOne:
uint256 schoolFees; // Missing in LevelTwo
uint256 public immutable reviewTime = 1 weeks; // Missing in LevelTwo
mapping(address => uint256) private reviewCount; // Missing in LevelTwo
mapping(address => uint256) private lastReviewTime; // Missing in LevelTwo

After upgrading to LevelTwo, these storage slots still exist but aren't properly defined in the code. Our testing demonstrates that:

  1. Functions from LevelOne that access these "missing" variables can still be called through the proxy

  2. If LevelTwo adds new variables that use these same storage slots, it will corrupt the undeclared variables

  3. This creates inconsistent behavior not reflected in the contract's code

Impact

The inconsistency between code and storage could lead to:

  1. Silent data corruption if LevelTwo uses the same storage slots for different purposes

  2. Unpredictable contract behavior as functions access "hidden" variables

  3. Long-term maintenance issues and increased risk with future upgrades

While access controls limit who can perform upgrades, the technical debt and potential for errors in future development remains significant.

Tools Used

Recommendations

  1. Maintain consistent storage layout: Ensure LevelTwo declares all state variables from LevelOne in the exact same order, even if they're no longer used:

  2. Add storage gaps: Include storage gaps in both contracts to reserve space for future variables

contract LevelTwo is Initializable {
// Must match LevelOne's storage layout exactly
address principal;
bool inSession;
uint256 schoolFees; // Keep this even if unused
uint256 public immutable reviewTime = 1 weeks;
// All other variables in the same order as LevelOne
// Only add new variables after all original ones
// Include a storage gap for future upgrades
uint256[50] private __gap;
}

POC

function test_UpgradeableImplementationVulnerability_WithImpact() public {
// Setup minimal state with no students (to avoid token transfers)
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
vm.stopPrank();
​
console.log("==================== BEFORE UPGRADE ====================");
// 1. Set a custom value for schoolFees directly through LevelOne
vm.prank(principal);
// Use vm.store to directly set schoolFees to avoid any function calls
bytes32 schoolFeesSlot = bytes32(uint256(1)); // schoolFees slot
uint256 customFees = 12345;
vm.store(address(levelOneProxy), schoolFeesSlot, bytes32(customFees));
​
// Verify our direct storage write worked
uint256 verifySchoolFees = levelOneProxy.getSchoolFeesCost();
console.log("Custom School Fees set in LevelOne:", verifySchoolFees);
assertEq(verifySchoolFees, customFees, "Direct storage write failed");
​
// 2. Now upgrade to LevelTwo WITHOUT calling graduateAndUpgrade
// (to avoid token transfers)
levelTwoImplementation = new LevelTwo();
​
// Directly set the implementation using the ERC1967 storage slot
bytes32 implSlot = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
vm.store(
address(levelOneProxy),
implSlot,
bytes32(uint256(uint160(address(levelTwoImplementation))))
);
​
// Call LevelTwo's initialize function directly
vm.prank(principal);
(bool success, ) = address(levelOneProxy).call(
abi.encodeWithSignature("graduate()")
);
​
console.log("==================== AFTER UPGRADE ====================");
console.log("Upgrade successful:", success);
​
// 3. Prove we are using LevelTwo by checking constants
LevelTwo levelTwoProxy = LevelTwo(address(levelOneProxy));
console.log(
"LevelTwo constant TEACHER_WAGE_L2:",
levelTwoProxy.TEACHER_WAGE_L2()
);
assertTrue(
levelTwoProxy.TEACHER_WAGE_L2() == 40,
"Not using LevelTwo implementation"
);
​
// 4. KEY PROOF: Directly read the schoolFees storage slot
// Even though LevelTwo doesn't declare this variable, the storage slot still exists!
bytes32 storedFeesBytes = vm.load(
address(levelOneProxy),
schoolFeesSlot
);
uint256 storedFees = uint256(storedFeesBytes);
console.log(
"Reading schoolFees storage slot after upgrade:",
storedFees
);
assertEq(
storedFees,
customFees,
"schoolFees was not preserved in storage"
);
​
// 5. Prove the vulnerability: we can modify this undeclared variable
uint256 corruptedValue = 999;
console.log("Corrupting schoolFees storage slot with:", corruptedValue);
vm.store(
address(levelOneProxy),
schoolFeesSlot,
bytes32(corruptedValue)
);
​
// Verify corruption
bytes32 corruptedBytes = vm.load(
address(levelOneProxy),
schoolFeesSlot
);
uint256 corruptedFees = uint256(corruptedBytes);
console.log("Value after corruption:", corruptedFees);
assertEq(corruptedFees, corruptedValue, "Failed to corrupt storage");
​
console.log("==================== IMPACT ====================");
console.log("Original schoolFees:", customFees);
console.log("Corrupted schoolFees:", corruptedFees);
console.log(
"This proves that a key state variable is still accessible in"
);
console.log("storage but not declared in the LevelTwo implementation.");
console.log(
"If LevelTwo uses this same storage slot for a different purpose,"
);
console.log("it would corrupt the original schoolFees variable.");
​
console.log("==================== CONCLUSION ====================");
console.log("STORAGE LAYOUT VULNERABILITY CONFIRMED:");
console.log(
"1. After upgrading to LevelTwo, the schoolFees storage slot still exists"
);
console.log(
"2. LevelTwo doesn't declare the schoolFees variable, creating a mismatch"
);
console.log(
"3. The variable can be corrupted if LevelTwo uses the same storage slot"
);
}


Updates

Lead Judging Commences

yeahchibyke Lead Judge about 1 month 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.