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

[EVMN-HH10] Critical Storage Compatibility Issue

evmninja

Summary

When upgrading from LevelOne to LevelTwo, there are serious storage layout inconsistencies that will cause data corruption. Upgradeable contracts must maintain identical storage layouts to ensure state variables are accessed correctly after upgrade.

Vulnerability Details

The following variables from LevelOne do not exist in LevelTwo:

  • uint256 public immutable reviewTime = 1 weeks; (line 41)

  • mapping(address => uint256) private reviewCount; (line 48)

  • mapping(address => uint256) private lastReviewTime; (line 49)

This storage layout mismatch will cause catastrophic data corruption after upgrade:

  • All variables after inSession will be shifted

  • LevelTwo will read incorrect data from wrong storage slots

  • Student scores, teacher lists, and other data will be completely corrupted

  • The entire system state will become unreliable and unpredictable

In Solidity's storage layout, variables are allocated sequential slots. When upgrading, if you remove variables from the middle of the sequence, all subsequent variables will read from incorrect slots.

For instance, where LevelOne stores reviewTime, LevelTwo will be reading sessionEnd data, creating a cascade of misaligned reads throughout the contract.

Impact

Critical (High impact, high likelihood)

Tools Used

Manual review.

Recommendations

The LevelTwo contract must declare exactly the same storage variables in the same order as LevelOne:

contract LevelTwo is Initializable {
using SafeERC20 for IERC20;
​
address principal;
bool inSession;
uint256 schoolFees; // Added missing variable
uint256 public immutable reviewTime = 1 weeks; // Add back the immutable (doesn't affect storage layout but keeps consistency)
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 missing variable
mapping(address => uint256) private lastReviewTime; // Added missing variable
address[] listOfStudents;
address[] listOfTeachers;
​
uint256 public constant TEACHER_WAGE_L2 = 40;
uint256 public constant PRINCIPAL_WAGE_L2 = 5;
uint256 public constant PRECISION = 100;
​
IERC20 usdc;
​
// Rest of the contract remains the same
}
Updates

Lead Judging Commences

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.