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

State Variables Not Initialized in LevelTwo Contract

linxun

Summary

The LevelTwo contract defines a graduate() function that serves as the reinitializer for the upgraded implementation, but it is empty and does not properly initialize critical state variables. This oversight will cause the contract to operate with default values (zero values) after upgrade instead of carrying forward the necessary state from LevelOne.

Vulnerability Details

The LevelTwo contract includes a graduate() function marked with reinitializer(2) which should properly initialize the contract state after an upgrade, but the function body is empty:

// @audit-medium stable variaties not initialized
function graduate() public reinitializer(2) {}

According to the OpenZeppelin documentation and best practices for upgradeable contracts, the reinitializer function should initialize all storage variables that will be used in the new implementation. Failing to do so leads to several issues:

  1. Critical state variables like principal, cutOffScore, and others will revert to their default values

  2. Access control will be broken since principal will be set to address(0)

  3. The upgraded contract will not maintain the state necessary for its functionality

This is further compounded by the fact that there is a storage slot inconsistency between LevelOne and LevelTwo. Specifically, LevelTwo is missing several state variables that exist in LevelOne (such as schoolFees and mappings for reviewCount and lastReviewTime), causing a misalignment in storage slots. This means that even if data remains in the proxy's storage after upgrade, it won't be correctly interpreted by the new implementation.

Impact

This issue has the following consequences:

  1. Loss of critical contract state after upgrade, including principal address and access controls

  2. Core functionality of the upgraded contract will be broken

  3. Security mechanisms based on properly initialized state variables will fail

  4. The upgrade process will not meet the project's invariants and requirements

The severity is assessed as Medium rather than High because while it poses significant operational issues, it doesn't directly lead to fund loss since the issue would be discovered immediately upon upgrade before critical operations can occur.

Tools Used

Manual code review

Recommendation

Implement proper initialization logic in the graduate() function to fulfill the project's requirements for system upgrade and student graduation. The function should address both state variable maintenance and business logic requirements:

function graduate() public reinitializer(2) {
// First, verify that the caller is authorized (should be the proxy contract during upgrade)
// This is important to prevent unauthorized calls to the initializer
// Note: This assumes the storage layout inconsistency is fixed first
// (LevelTwo is missing variables like schoolFees, reviewCount, etc. from LevelOne)
// to ensure state variables are in the correct storage slots
// Filter graduating students based on cutoff score as required by the invariant
address[] memory qualifiedStudents = new address[](listOfStudents.length);
uint256 qualifiedCount = 0;
for (uint256 i = 0; i < listOfStudents.length; i++) {
address student = listOfStudents[i];
// Only graduate students who meet the cutoff score
if (studentScore[student] >= cutOffScore) {
qualifiedStudents[qualifiedCount] = student;
qualifiedCount++;
} else {
// Reset student status for those who didn't qualify
isStudent[student] = false;
}
}
// Replace student list with only qualified students
delete listOfStudents;
for (uint256 i = 0; i < qualifiedCount; i++) {
listOfStudents.push(qualifiedStudents[i]);
}
// Reset session-related variables for the next term
inSession = false;
sessionEnd = 0;
// Maintain 60% of the bursary as required by the invariant
// (Note: this assumes the bursary value is already correctly stored in the proxy
// and accessible to this contract after fixing the storage layout inconsistency)
// Reset review-related variables for the new term
// This requires adding the missing mappings that are in LevelOne but missing in LevelTwo:
// mapping(address => uint256) private reviewCount;
// mapping(address => uint256) private lastReviewTime;
emit Graduated(address(this));
}
​
// Additionally, implement a setter function to help with emergency recovery
// in case the storage slots are misaligned:
function setStateVariables(
address _principal,
uint256 _bursary,
uint256 _cutOffScore
) external {
// Add appropriate access control
// This function should only be callable by an admin right after upgrade
principal = _principal;
bursary = _bursary;
cutOffScore = _cutOffScore;
}
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.