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

Critical Access Control Vulnerability in LevelTwo's Reinitialization Function

umarbinkhattab

Issue Summary

The LevelTwo contract contains a graduate() function marked with the reinitializer(2) modifier but lacks any access control mechanisms. This critical security oversight allows any address to call this privileged function, which should be restricted to authorized system administrators (like the principal) or the proxy contract during an upgrade process.

Impact - Critical

This vulnerability completely undermines the security model of the upgradeable contract system. The unrestricted graduate() function creates several severe risks:

  1. Unauthorized System Reinitialization: Any external actor can trigger the reinitialization process, potentially corrupting the system state

  2. Upgrade Hijacking: Malicious actors can front-run legitimate upgrade transactions by calling this function first

  3. Denial of Service: Since the reinitializer can only be called once, attackers can permanently block legitimate initialization by calling it first

  4. Financial Risk: Given that bursary funds are carried over from LevelOne, unauthorized control of reinitialization could potentially redirect or lock these funds

Likelihood - High

The exploitation likelihood is high because:

  1. The vulnerability requires minimal technical expertise to exploit (single function call)

  2. The function is public with no parameters, presenting a trivial attack vector

  3. No access controls exist whatsoever to prevent unauthorized calls

  4. The function is named in an intuitive way that indicates its purpose

Technical Details

The vulnerable implementation in the LevelTwo contract:

function graduate() public reinitializer(2) {}

Key security issues:

  1. No Access Modifier: Unlike other administrative functions in both LevelOne and LevelTwo contracts, this critical function lacks any access control modifier

  2. Empty Implementation: The function body is empty, suggesting it's meant to be a placeholder for initialization logic during an upgrade

  3. Reinitializer Version: The reinitializer(2) indicates this is for the second initialization after an upgrade from LevelOne

When compared to the LevelOne contract's initialization function:

function initialize(address _principal, uint256 _schoolFees, address _usdcAddress) public initializer {
if (_principal == address(0)) {
revert HH__ZeroAddress();
}
// ...more validations and initializations
}

The LevelTwo contract's graduate() function lacks both the necessary access controls and implementation logic.

Proof of Concept

A simple attack scenario:

  1. LevelOne contract is deployed and operational with students, teachers, and accumulated bursary funds

  2. The system administrator prepares for an upgrade to LevelTwo

  3. Before the legitimate upgrade transaction is processed, an attacker monitors the mempool and calls graduate() on the LevelTwo implementation

  4. The reinitialization is marked as complete but with none of the required state migration or setup

  5. The legitimate upgrade fails or results in a corrupted system state

Remediation

Implement proper access control and initialization logic:

// Add error definition
error LevelTwo__NotAuthorized();
​
// Add modifier
modifier onlyPrincipal() {
if (msg.sender != principal) {
revert LevelTwo__NotAuthorized();
}
_;
}
​
// Properly implement the graduate function
function graduate() public onlyPrincipal reinitializer(2) {
// Perform necessary initialization logic
// e.g., setting up initial state, validating carried-over parameters
// Consider additional verification that this is being called in an upgrade context
}

For enhanced security in the upgrade process, consider one of these approaches:

  1. Use a specific upgrade manager role that's separate from day-to-day administration

  2. Implement a time-lock mechanism for upgrade operations

  3. Add validation to ensure the function is called in the context of a proxy upgrade transaction

These changes would ensure that only authorized entities can perform the critical reinitialization during the upgrade process.

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.