Hawk High

Severity

Medium

Impact

The contract lacks explicit reentrancy guards on functions that interact with external ERC20 tokens. While SafeERC20 is used, this doesn't protect against cross-function reentrancy attacks involving tokens with callbacks like ERC777 or certain deflationary tokens that might have been wrapped in an ERC20 interface.

Description

The enroll() and graduateAndUpgrade() functions perform token transfers but don't follow the Checks-Effects-Interactions pattern, leaving them potentially vulnerable to reentrancy.

Potential Attack Scenario in graduateAndUpgrade()

This vulnerability could be exploited as follows:

1. A malicious actor gets added as a teacher (possibly through social engineering or a compromised principal)

2. They deploy a malicious contract with callback functionality that serves as their "teacher" address

3. When graduateAndUpgrade() is called:

   • The malicious implementation is authorized via _authorizeUpgrade(_levelTwo)

   • The contract makes external token transfers to teachers

   • When paying the malicious teacher contract, its callback function activates

   • The callback could manipulate the contract state during this vulnerable transitional period:

     // Example malicious teacher contract

     contract MaliciousTeacher {

     LevelOne school;

     constructor(address _school) {

     school = LevelOne(_school);

     }

     // Callback triggered during token transfer

     function tokensReceived(address, address, uint256) external {

     // Execute malicious actions during callback

     // Possibly steal funds or manipulate state

     // This runs when the contract is in a partially upgraded state

     }

     }

4. Since the UUPS upgrade is already authorized but transfers are still executing from the old implementation, this creates a unique attack window

Additionally, the enroll() function has similar reentrancy concerns:

While standard USDC doesn't implement callbacks, if the contract is deployed on chains where USDC is implemented differently or if Circle upgrades the USDC implementation to include callback mechanisms in the future, this could expose the contract to risk.

Recommended Mitigation

1. Add a reentrancy guard

2. Reorder operations to follow Checks-Effects-Interactions pattern

3. Most critically, ensure the contract upgrade in graduateAndUpgrade() happens AFTER all token transfers to prevent reentrancy during this critical operation