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

H-01: Critical Storage Layout Mismatch in `LevelTwo` Corrupts State Upon Upgrade

albert

Summary

LevelTwo.sol does not maintain the same storage layout as LevelOne.sol by omitting several state variables (schoolFees, reviewCount, lastReviewTime). This mismatch will cause state corruption upon upgrade, as LevelTwo will read data from incorrect storage slots, rendering it non-functional and leading to data integrity loss.

Vulnerability Details

The UUPS proxy pattern requires that the new implementation contract preserves the storage layout of the previous contract for all inherited state variables. LevelOne.sol declares state variables like schoolFees, reviewCount, and lastReviewTime. LevelTwo.sol omits these. When an upgrade occurs, the storage slots are interpreted by the new contract's layout. For example:

LevelOne storage order (simplified):
0. principal (address)

  1. inSession (bool)

  2. schoolFees (uint256)

  3. sessionEnd (uint256) (immutable reviewTime doesn't occupy a standard storage slot here)

  4. bursary (uint256)

  5. cutOffScore (uint256)

  6. isTeacher (mapping)

  7. isStudent (mapping)

  8. studentScore (mapping)

  9. reviewCount (mapping)

  10. lastReviewTime (mapping)

  11. listOfStudents (array)

  12. listOfTeachers (array)

  13. usdc (address)

If LevelTwo omits schoolFees, then its sessionEnd variable (if it's the second uint256 after inSession in LevelTwo's declaration) would try to read from slot 2, which actually holds LevelOne.schoolFees. This misalignment continues for subsequent variables, corrupting all inherited state.

Impact

State corruption upon upgrade will render LevelTwo non-functional. Its state variables will hold entirely incorrect values, breaking all logic within LevelTwo. This can lead to a complete loss of data integrity for critical information such as the bursary amount, studentScores, sessionEnd times, and teacher/student lists. The school system would become unusable and funds could be misinterpreted or locked.

Tools Used

Manual Review, Understanding of EVM storage layout and UUPS proxy patterns.

Recommendations

Ensure LevelTwo.sol declares all state variables in the exact same order and type as LevelOne.sol up to the last variable of LevelOne. If variables like schoolFees, reviewCount, and lastReviewTime are intentionally deprecated for LevelTwo's logic, they must still be declared (e.g., as private) to preserve the storage layout. New state variables for LevelTwo must be appended after all of LevelOne's variables.

Code Modification for LevelTwo.sol (State Variable Section):

// src/LevelTwo.sol
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Initializable} from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract LevelTwo is Initializable {
using SafeERC20 for IERC20;
​
// --- START OF MODIFICATION FOR H-01 ---
// Variables from LevelOne to maintain storage layout compatibility
address principal;
bool inSession;
uint256 schoolFees; // ADDED FOR LAYOUT COMPATIBILITY (was missing)
// uint256 public immutable reviewTime; // Immutables don't take up standard storage slots for proxy state
uint256 public sessionEnd;
uint256 public bursary;
uint256 public cutOffScore;
mapping(address => bool) public isTeacher;
mapping(address => bool) public isStudent;
mapping(address => uint256) public studentScore;
mapping(address => uint256) private reviewCount; // ADDED FOR LAYOUT COMPATIBILITY (was missing)
mapping(address => uint256) private lastReviewTime; // ADDED FOR LAYOUT COMPATIBILITY (was missing)
address[] listOfStudents;
address[] listOfTeachers;
// IERC20 usdc; // Declared below, ensure it's in the correct final position.
​
// LevelTwo specific constants (Original)
uint256 public constant TEACHER_WAGE_L2 = 40;
uint256 public constant PRINCIPAL_WAGE_L2 = 5;
uint256 public constant PRECISION = 100;
​
IERC20 usdc; // This should align with LevelOne's usdc position
// --- END OF MODIFICATION FOR H-01 ---
​
​
function graduate() public reinitializer(2) {}
​
function getPrincipal() external view returns (address) {
return principal;
}
​
function getSchoolFeesToken() external view returns (address) {
return address(usdc);
}
​
function getTotalTeachers() external view returns (uint256) {
return listOfTeachers.length;
}
​
function getTotalStudents() external view returns (uint256) {
return listOfStudents.length;
}
​
function getListOfStudents() external view returns (address[] memory) {
return listOfStudents;
}
​
function getListOfTeachers() external view returns (address[] memory) {
return listOfTeachers;
}
}
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.