Hawk High

Severity: High

Likelihood: High

Summary

The LevelOne and LevelTwo contracts exhibit a fundamental storage layout incompatibility that will cause catastrophic data corruption upon upgrade. The inconsistent variable declaration patterns and missing variables in LevelTwo create a perfect storm for silent storage corruption. This report provides an in-depth technical analysis of how this vulnerability affects all contract state variables and the far-reaching implications for contract integrity, focusing particularly on subtle corruption vectors that may go undetected initially.

Vulnerability Detail

Comprehensive Storage Layout Comparison

Let's first understand the exact storage layout of both contracts, slot by slot:

LevelOne's Storage Layout:

LevelTwo's Storage Layout:

Technical Analysis of Storage Collision Effects

1. Variable-by-Variable Collision Impact Analysis

Principal and inSession Slots (0-1):

• While these align correctly, they offer a false sense of security that masks deeper issues.

SchoolFees to SessionEnd Collision (Slot 2):

• LevelOne stores the schoolFees variable in slot 2.

• LevelTwo uses slot 2 for sessionEnd.

• After upgrade, LevelTwo's sessionEnd will contain the value of schoolFees from LevelOne.

• Impact: Session timing based on a monetary value, potentially resulting in premature or severely delayed session endings.

• Technical consequence: If schoolFees is 50 USDC (5000000000 in wei scale), sessionEnd would be interpreted as a timestamp 5000000000 seconds after the Unix epoch (around 2128), causing sessions to never end.

SessionEnd to Bursary Collision (Slots 3-4):

• LevelOne's sessionEnd (timestamp) will become LevelTwo's bursary (monetary value).

• Impact: The treasury amount will be incorrectly set to a timestamp value.

• Technical consequence: If sessionEnd was May 2025 (~1746144000), this would represent a massive and incorrect bursary balance of 1,746,144,000 tokens.

CutOffScore and Mapping Collisions (Slots 5-8):

• LevelOne's cutOffScore will overwrite the storage slot that's supposed to be a mapping pointer for isTeacher in LevelTwo.

• Technical consequence: If cutOffScore was 70, the isTeacher mapping would point to storage location 70, reading completely arbitrary data.

Mapping Pointer Corruption (Slots 6-10):

• Each mapping uses its declared slot as a pointer to where its data actually begins.

• When these slots are misaligned, the mappings will point to completely wrong storage areas.

• Technical consequence: For example, LevelTwo's isStudent mapping would use the pointer previously used for LevelOne's isTeacher mapping, causing it to read teacher data as student data.

2. Deep Storage Analysis of Mapping Collisions

Mapping storage in Solidity follows this pattern:

• For a mapping at slot p, the value for key k is stored at keccak256(h(k) . p) where h is a function that pads the key to 32 bytes.

This means:

• LevelOne's isTeacher mapping: Values stored at keccak256(h(address) . 6)

• LevelTwo's isTeacher mapping: Will look at keccak256(h(address) . 5)

The consequence is profound:

• LevelTwo will never access the correct teacher data after upgrade.

• Instead, it will access completely unrelated storage locations determined by the new hash calculation.

• This creates both a data loss issue (can't retrieve original data) and a data corruption issue (reading arbitrary data).

3. Array Storage Structure Breakdown

Dynamic arrays in Solidity have a unique storage pattern:

• At the declared slot s: The array length is stored

• Actual elements start at keccak256(s)

After upgrade:

• listOfStudents moves from slot 11 to slot 8

• LevelTwo will read the length from slot 8, which contained LevelOne's mapping pointer for studentScore

• The array elements will be read from keccak256(8), which is completely different from keccak256(11)

• This will likely interpret a mapping pointer as an array length (potentially a very large value) and then try to read array elements from incorrect locations.

4. Complex Data Type Access Patterns

For nested mappings and arrays of structs, the corruption becomes exponentially more complex:

• If LevelOne had mapping(address => mapping(uint256 => bool)), the collision in LevelTwo would cause multi-dimensional hash miscalculations.

• This creates unpredictable storage access patterns that are virtually impossible to diagnose or fix after the fact.

Impact

The storage collision creates a cascade of corrupted data that affects every aspect of the contract's operation:

1. Financial System Integrity Collapse

• Treasury Mismanagement: The bursary variable will contain LevelOne's sessionEnd timestamp value after upgrade, potentially representing millions or billions of tokens rather than the actual balance.

• Payment Calculation Errors: Teacher and principal payments are calculated as percentages of the bursary. With a corrupted bursary value, payments would be astronomically inflated, potentially draining all contract funds in a single transaction.

2. Access Control Compromise

• Role Confusion: The isTeacher and isStudent mappings will be reading from incorrect storage locations.

• Security Breach: This could allow students to gain teacher privileges or vice versa, compromising the entire permission system.

• Authentication Bypass: A user who was neither a teacher nor a student in LevelOne might suddenly have permissions in LevelTwo due to arbitrary storage data being interpreted as role flags.

3. Academic Record Corruption

• Score Integrity Loss: Student scores will be completely corrupted, with values potentially reading from arbitrary storage locations.

• Scholarship Eligibility Errors: Decisions based on student performance would use completely incorrect data.

• Graduation Criteria Failure: The cutoff score system would be operating on meaningless data.

4. Operational Paralysis

• Session Management Failure: sessionEnd will contain the value of schoolFees, likely resulting in sessions that never end or end immediately.

• Student Record Chaos: Attempting to access student records through listOfStudents would either revert due to out-of-gas (if the corrupted length is extremely large) or return completely wrong addresses.

• Payment Distribution Failure: Any attempt to distribute payments to teachers would either revert or send funds to incorrect addresses.

5. Systemic Technical Consequences

• Silent Failures: Many of these issues won't cause reverts but will silently corrupt data, making them particularly insidious.

• Data Retrieval Impossibility: Original data becomes inaccessible but still consumes storage (and cost) in the contract.

• Irreversible Damage: Once the upgrade occurs, there is no clean way to recover the original data structure.

Code Reference

The vulnerability spans the entire storage layout of both contracts. Here's a detailed side-by-side comparison highlighting the critical mismatches:

The graduate() function in LevelTwo is particularly concerning as it does nothing to address these storage mismatches:

This function, despite using OpenZeppelin's reinitializer modifier to indicate a new initialization, fails to actually initialize or align any storage variables to maintain compatibility with LevelOne's data.

Recommendation

1. Preserve Exact Storage Layout

The LevelTwo contract must declare all variables from LevelOne in the exact same order:

2. Implement Robust Storage Gap Pattern

Add storage gaps to both contracts to ensure future upgrades have space for new variables:

3. Use OpenZeppelin's Storage Layout Tools

Integrate with OpenZeppelin's storage layout tooling to detect and prevent such issues:

4. Utilize Storage Inheritance Pattern

Rather than duplicating storage declarations, create a shared base contract for storage:

5. Document Storage Layout as Architecture

Create a formal storage layout document that:

• Maps each variable to its slot

• Specifies the intended data type and usage

• Logs any changes between versions

• Must be reviewed and approved before any upgrade