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

Missing Access Control in LevelTwo's Graduate Function

khalidwalid

Issue Description

The graduate function in the LevelTwo contract is implemented with a reinitializer(2) modifier but lacks any access control mechanism. This means any address can call this function, which should logically be restricted to authorized roles like the principal, as it represents a critical system transition.

Impact - Critical

Unrestricted access to the graduate function allows any external actor to trigger the reinitialization of the contract. This can lead to unauthorized graduation procedures, potential manipulation of system state, and disruption of the entire educational platform's operation. Since this function is designed to be the entry point for contract reinitialization during an upgrade, unauthorized access could result in complete system hijacking.

Likelihood - High

The likelihood of exploitation is high because:

  1. The function is public with no access restrictions

  2. It has a simple signature with no parameters, making it easy to call

  3. The reinitializer(2) modifier only prevents multiple initializations but doesn't restrict who can perform the initialization

Detailed Analysis

The vulnerable code in LevelTwo contract:

function graduate() public reinitializer(2) {}

Comparing with LevelOne contract's access-controlled functions:

// LevelOne's initialization function
function initialize(address _principal, uint256 _schoolFees, address _usdcAddress) public initializer {
// ...
}
​
// LevelOne's upgrade-related function
function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
// ...
}

The LevelOne contract appropriately uses the initializer modifier for its initialization function and restricts upgrade functionality with onlyPrincipal. In contrast, LevelTwo's graduate function only uses reinitializer(2) which ensures the function can only be called once but places no restrictions on who can call it.

OpenZeppelin's reinitializer only prevents multiple initializations but does not provide any access control:

modifier reinitializer(uint8 version) {
require(!_initializing && _initialized < version, "Initializable: contract is already initialized");
_initialized = version;
_initializing = true;
_;
_initializing = false;
emit Initialized(version);
}

This creates a situation where any external address can trigger the graduation process, which should logically be restricted to an authorized role.

Recommendation

Implement proper access control on the graduate function:

// Add modifier
modifier onlyPrincipal() {
require(msg.sender == principal, "LevelTwo: caller is not the principal");
_;
}
​
// Fix the vulnerable function
function graduate() public onlyPrincipal reinitializer(2) {
// Implementation
}

Alternatively, if the function is meant to be called only by the proxy during an upgrade, it should implement logic to verify it's being called in the context of a proxy upgrade transaction.

Updates

Lead Judging Commences

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