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

L-02: `LevelTwo.graduate()` Reinitializer is Empty and `LevelTwo` Lacks Proper UUPS Initialization Logic

albert

Summary

LevelTwo.sol has an empty reinitializer(2) function named graduate(). Additionally, if LevelTwo is intended to be UUPS-upgradeable itself, it doesn't call __UUPSUpgradeable_init_unchained() or a similar function. This can lead to confusion and a lack of proper initialization post-upgrade or for subsequent upgrades.

Vulnerability Details

The graduate() function in LevelTwo.sol is declared as function graduate() public reinitializer(2) {}.

  1. It's empty, providing no re-initialization logic for LevelTwo's state after it becomes the active implementation.

  2. LevelTwo inherits Initializable but does not have its own initializer (with the initializer modifier). If it's the target of an upgrade from LevelOne, its state (like principal, usdc, cutOffScore specific to LevelTwo rules, and filtered student list) needs to be set up. The reinitializer is the place for this.

  3. If LevelTwo itself is intended to be upgradeable further via UUPS, its UUPS capabilities (like setting its own _owner for _authorizeUpgrade) are not initialized because __UUPSUpgradeable_init_unchained() (or __UUPSUpgradeable_init if it were a standalone deployment) is not called.

Impact

  • Confusion & Missing Logic: The empty graduate() function is misleading. Key state variables in LevelTwo (e.g., its own principal, cutOffScore if different, or filtered listOfStudents as per H-04) might not be set correctly after the upgrade from LevelOne.

  • Non-Upgradeable LevelTwo (Potentially): If LevelTwo is meant to be further upgradeable via UUPS, it won't be properly set up to authorize future upgrades without calling __UUPSUpgradeable_init_unchained().

Tools Used

Manual Review, Understanding of OpenZeppelin Initializable and UUPSUpgradeable patterns.

Recommendations

  1. Implement the necessary state initialization logic within the graduate(args...) reinitializer in LevelTwo. This function should accept parameters (passed via dataForLevelTwoInitialize from LevelOne) to set up LevelTwo's specific state, such as its principal, usdc token address (if it could change), and cutOffScore.

  2. Critically, the graduate() reinitializer should handle the logic for H-04 (Part 3): filtering listOfStudents (inherited from LevelOne's storage) based on studentScore and cutOffScore to ensure only eligible students are part of LevelTwo.

  3. If LevelTwo is intended to be UUPS-upgradeable itself, call __UUPSUpgradeable_init_unchained() within its reinitializer. The version for reinitializer (e.g., reinitializer(2)) should be higher than LevelOne's initializer version.

Code Modification for LevelTwo.sol::graduate (Illustrative, assuming H-01 fixed):

// src/LevelTwo.sol
// (Ensure H-01 is fixed by declaring all LevelOne state variables first for layout compatibility)
contract LevelTwo is Initializable { // Add UUPSUpgradeable if LevelTwo is also to be upgradeable.
// E.g., contract LevelTwo is Initializable, UUPSUpgradeable {
using SafeERC20 for IERC20;
​
// ... (All state variables from LevelOne in order - SEE H-01) ...
// address principal;
// bool inSession;
// uint256 schoolFees; // From LevelOne for layout
// uint256 public sessionEnd;
// uint256 public bursary;
// uint256 public cutOffScore; // Will be set by reinitializer
// mapping(address => bool) public isTeacher;
// mapping(address => bool) public isStudent;
// mapping(address => uint256) public studentScore;
// mapping(address => uint256) private reviewCount; // From LevelOne for layout
// mapping(address => uint256) private lastReviewTime; // From LevelOne for layout
// address[] listOfStudents; // Inherited from LevelOne, will be filtered
// address[] listOfTeachers;
// IERC20 usdc;
​
uint256 public constant TEACHER_WAGE_L2 = 40; // LevelTwo specific
uint256 public constant PRINCIPAL_WAGE_L2 = 5; // LevelTwo specific
uint256 public constant PRECISION = 100;
​
// --- START OF MODIFICATION FOR L-02 (and H-04 Part 3) ---
// This function is called by LevelOne's _upgradeToAndCallUUPS
// Parameters are passed via `dataForLevelTwoInitialize`
function graduate(address _newPrincipal, address _usdcAddress, uint256 _levelOneCutOffScore) public reinitializer(2) {
// If LevelTwo itself is intended to be UUPS upgradeable by _newPrincipal:
// __UUPSUpgradeable_init_unchained(); // Initializes UUPS for LevelTwo.
// If UUPSUpgradeable is added to contract definition, you'd also need:
// _transferOwnership(_newPrincipal); // or however UUPS owner is set for LevelTwo
​
principal = _newPrincipal;
usdc = IERC20(_usdcAddress);
cutOffScore = _levelOneCutOffScore; // Use the cutoff score from LevelOne for this graduation
inSession = false; // A new session for LevelTwo would need to be started explicitly
// schoolFees might be re-defined or deprecated for LevelTwo. If used, initialize.
​
// Logic for H-04 Part 3: Filter students based on cutOffScore from LevelOne
// listOfStudents and studentScore are inherited from LevelOne's state.
address[] memory studentsFromLevelOneStorage = listOfStudents;
delete listOfStudents; // Clear LevelTwo's list to rebuild with only graduated students
​
for (uint256 i = 0; i < studentsFromLevelOneStorage.length; i++) {
address student = studentsFromLevelOneStorage[i];
// isStudent[student] and studentScore[student] are from LevelOne's state
if (isStudent[student] && studentScore[student] >= cutOffScore) {
listOfStudents.push(student);
// studentScore[student] is already correct from LevelOne.
// isStudent[student] remains true for these graduated students.
} else {
// Student does not graduate to LevelTwo.
// Mark them as not a student in LevelTwo's context.
isStudent[student] = false;
}
}
​
// Teachers list (listOfTeachers) is also inherited. Principal can manage it in LevelTwo.
// Or, it can be reset here if LevelTwo starts with no teachers: delete listOfTeachers;
}
// --- END OF MODIFICATION FOR L-02 ---
​
// ... (getter functions as originally in LevelTwo) ...
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 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

failed upgrade

The system doesn't implement UUPS properly.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.