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

Storage Collision in LevelTwo Due to Removed schoolFees Variable

ddeenny

Summary

The LevelTwo contract omits the schoolFees variable that exists in LevelOne, breaking the expected storage layout. Since Solidity stores state variables sequentially in storage slots, removing or reordering inherited variables in an upgradeable contract leads to storage collisions, where new variables overwrite existing data. This can cause unpredictable behavior, including fund misrouting, broken access control, or permanent data corruption.

Vulnerability Details

// LevelOne layout
address principal; // slot 0
bool inSession; // slot 1
uint256 schoolFees; // slot 2
IERC20 usdc; // slot 3
​
// LevelTwo layout (schoolFees missing!)
address principal; // slot 0
bool inSession; // slot 1
// @audit-issue now incorrectly occupies slot 2
@> IERC20 usdc;

Issue Explanation

  1. Shared Storage via Proxy
    Upgradeable contracts share one storage layout defined by the proxy. Changing that layout in newer implementations causes serious inconsistencies.

  2. Slot Misalignment
    By removing schoolFees, the usdc variable is now occupying the original slot for schoolFees, and its expected value becomes corrupted.

  3. Downstream Corruption
    Calls to usdc.safeTransfer(...) will likely fail, misroute tokens, or interact with an invalid address.

Impact

  • Fund Corruption: Transfers using usdc may go to unintended addresses due to corrupted address values.

  • Broken Logic: Access control, balances, and other logic relying on overwritten slots will malfunction.

  • Irrecoverable State: Once data is corrupted by a storage collision, it cannot be restored.

Tools Used

  • Manual Code Review

Recommendations

To prevent storage collisions, preserve the exact variable layout from previous versions:

// LevelTwo (corrected layout)
address principal;
bool inSession;
uint256 schoolFees; // @fix preserved from LevelOne
uint256 public immutable reviewTime = 1 weeks;
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;
mapping(address => uint256) private lastReviewTime;
address[] listOfStudents;
address[] listOfTeachers;
​
uint256 public constant TEACHER_WAGE = 35;
uint256 public constant PRINCIPAL_WAGE = 5;
uint256 public constant PRECISION = 100;
​
IERC20 usdc;

If new variables are needed, append them after all existing ones.

Updates

Lead Judging Commences

yeahchibyke Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

storage collision

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