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

Storage Collision on upgrade from `LevelOne` to `LevelTwo` causes misaligned state variables

finn79426

Summary

The upgrade from LevelOne contract to LevelTwo contract introduces a storage layout mismatch that results in a critical storage collision. Specifically, variables such as schoolFees, sessionEnd, bursary and cutOffScore in LevelOne are removed or reordered in LevelTwo, leading to misalignment of subsequent storage slots. When the implementation slot in the ERC1967 proxy contract is upgraded to LevelTwo, previously initialized storage slots will be interpreted incorrectly, potentially causing incorrect contract behavior or fund mismanagement.

Vulnerability Details

In Solidity, storage layout is strictly positional and upgrades must maintain storage variable order and structure to avoid corruption. The V1 contract contains several state variables that are removed or restructured in V2:

  • LevelOne includes schoolFees, reviewCount and lastReviewTime.

  • LevelTwo removes these variables entirely.

  • LevelOne includes studentScore, reviewCount and lastReviewTime.

  • LevelTwo removes these mappings entirely.

  • LevelOne includes sessionEnd, bursary and cutOffScore.

  • LevelTwo moved these variables up, each variable has moved up one slot. (ie. from slot2 to slot1)

  • LevelOne includes isTeacher, isStudent, and studentScore.

  • LevelTwo moved these mappings up, each mapping has moved up one slot. (ie. from slot2 to slot1)

  • LevelOne includes listOfStudents and listOfTeachers.

  • LevelTwo moved these dynamic arrays up, each array has moved up one slot. (ie. from slot2 to slot1)

This kind of misalignment can lead to:

  • Corrupted logic based on incorrect state values (e.g., sessionEnd reading a leftover schoolFees value),

  • Broken access control or roles (due to incorrect isTeacher/isStudent values),

  • Financial errors in bursary distribution or wage calculation.

Proof of Concept

In order to demonstrate the impact of this vulnerability, we need to slightly modify the original source code:

// Step1: Add this getUSDC() function into LevelOne and LevelTwo contract.
// src/LevelOne.sol
// src/LevelTwo.sol
function getUSDC() external view returns (address) {
return address(usdc);
}
// Step2: Fix broken UUPS upgrade logic.
// src/LevelOne.sol
function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
​
uint256 totalTeachers = listOfTeachers.length;
​
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
​
// Fix here:
// _authorizeUpgrade(_levelTwo);
upgradeToAndCall(_levelTwo, "");
​
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
​
usdc.safeTransfer(principal, principalPay);
}
// Step3: Add the below codes into the LevelTwo contract.
// src/LevelTwo.sol
import {ERC1967Utils} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Utils.sol";
​
function proxiableUUID() external view virtual returns (bytes32) {
return ERC1967Utils.IMPLEMENTATION_SLOT;
}

Then, we slightly modify the test_confirm_can_graduate() test function in order to see how storage collision impacts.

function test_confirm_can_graduate() public schoolInSession {
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
​
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
console2.log(levelOneProxy.getUSDC()); // Add here
​
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
​
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
​
console2.log(levelTwoProxy.getUSDC()); // Add here
​
console2.log(levelTwoProxy.bursary());
console2.log(levelTwoProxy.getTotalStudents());
}

Finally, run the PoC:

╰─ forge test test/LeveOnelAndGraduateTest.t.sol --match-test test_confirm_can_graduate -vvv

You should be able to see the output below in your console:

Logs:
levelOneProxy.getUSDC() = 0x104fBc016F4bb334D775a19E8A6510109AC63E00
levelOneProxy.getUSDC() = 0x0000000000000000000000000000000000000000

After the upgrade, the variable usdc points to an unused storage slot, this will cause the contract to be Denial-Of-Service.

Impact

The impact is high because:

  • State corruption may result in incorrect session states, faulty role permissions, and misallocated funds.

  • It may break critical contract logic that relies on these state variables, potentially bricking the application or causing loss of user funds.

  • The upgrade would appear successful on-chain but the contract will silently misbehave post-upgrade.

This kind of vulnerability can undermine trust in the protocol and may require a forceful rollback or redeployment to resolve, both of which carry reputational and operational costs.

Tools Used

  • Manual Code Review

  • Foundry

  • Slither-Read-Storage

Recommendations

  • Never remove or reorder storage variables in upgradeable contracts. Instead, append new variables after existing ones.

  • Introduce a LevelTwoStorage struct and inherit it after LevelOneStorage to ensure layout preservation.

  • Include comprehensive upgrade tests that simulate the full upgrade from LevelOne to LevelTwo and validate expected state values.

  • Consider using a gap pattern (i.e., reserved storage slots) to allow for future-safe expansion without shifting existing variables.

Updates

Lead Judging Commences

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

Give us feedback!