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

Missing Review Count Validation in Graduation Process (Input Validation & Business Logic Vulnerability)

teheelaa

Summary

The graduateAndUpgrade function in LevelOne.sol lacks validation to ensure students have received all 4 required weekly reviews before graduation, allowing students to graduate without proper evaluation.

Vulnerability Details

The contract fails to enforce a core requirement that students must receive 4 weekly reviews before graduation. It can be found here:

​
function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
​
uint256 totalTeachers = listOfTeachers.length;
​
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
​
_authorizeUpgrade(_levelTwo);
​
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
​
usdc.safeTransfer(principal, principalPay);
}

  • The README explicitly states this requirement:

Students must have gotten all reviews before system upgrade. System upgrade should not occur if any student has not gotten 4 reviews (one for each week)

  • While there is a reviewCount mapping in LevelOne.sol, it's only used in the giveReview function to prevent more than 5 reviews, but it's not checked during graduation:

require(reviewCount[_student] < 5, "Student review count exceeded!!!");

POC

Here's a proof of concept that demonstrates this vulnerability:

// 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 MissingReviewValidationTest is Test {
DeployLevelOne deployBot;
GraduateToLevelTwo graduateBot;
LevelOne levelOneProxy;
LevelTwo levelTwoImplementation;
MockUSDC usdc;
​
address proxyAddress;
address levelOneImplementationAddress;
address principal;
uint256 schoolFees;
​
// Teachers
address alice = makeAddr("alice");
address bob = makeAddr("bob");
​
// Students
address clara = makeAddr("clara");
address dan = makeAddr("dan");
​
function setUp() public {
// Deploy contracts
deployBot = new DeployLevelOne();
proxyAddress = deployBot.deployLevelOne();
levelOneProxy = LevelOne(proxyAddress);
​
// Get contract instances and variables
usdc = deployBot.getUSDC();
principal = deployBot.principal();
schoolFees = deployBot.getSchoolFees();
levelOneImplementationAddress = deployBot.getImplementationAddress();
​
// Create addresses
clara = makeAddr("clara");
dan = makeAddr("dan");
alice = makeAddr("alice");
bob = makeAddr("bob");
​
// Setup initial USDC balances
vm.startPrank(address(usdc));
usdc.mint(clara, schoolFees);
usdc.mint(dan, schoolFees);
vm.stopPrank();
}
​
function test_POC_GraduateWithoutRequiredReviews() public {
// 1. Add teachers
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
levelOneProxy.addTeacher(bob);
vm.stopPrank();
​
// 2. Enroll students
vm.startPrank(clara);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
vm.startPrank(dan);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
// 3. Start school session with cutoff score of 70
vm.prank(principal);
levelOneProxy.startSession(70);
​
// 4. Give only 2 reviews to students (instead of required 4)
vm.startPrank(alice);
vm.warp(block.timestamp + 1 weeks);
levelOneProxy.giveReview(clara, true); // Good review
levelOneProxy.giveReview(dan, true);
​
vm.warp(block.timestamp + 1 weeks);
levelOneProxy.giveReview(clara, true);
levelOneProxy.giveReview(dan, true);
vm.stopPrank();
​
// 5. Fast forward to session end
vm.warp(block.timestamp + 2 weeks);
​
// 6. Deploy LevelTwo and attempt graduation
levelTwoImplementation = new LevelTwo();
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
// Store balances before graduation
uint256 aliceBalanceBefore = usdc.balanceOf(alice);
uint256 bobBalanceBefore = usdc.balanceOf(bob);
uint256 principalBalanceBefore = usdc.balanceOf(principal);
​
// Graduate despite incomplete reviews
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(address(levelTwoImplementation), data);
​
// Check balances after graduation
uint256 aliceBalanceAfter = usdc.balanceOf(alice);
uint256 bobBalanceAfter = usdc.balanceOf(bob);
uint256 principalBalanceAfter = usdc.balanceOf(principal);
​
console2.log(
"Alice balance change:",
aliceBalanceAfter - aliceBalanceBefore
);
console2.log("Bob balance change:", bobBalanceAfter - bobBalanceBefore);
console2.log(
"Principal balance change:",
principalBalanceAfter - principalBalanceBefore
);
​
// Check if students are in LevelTwo
LevelTwo levelTwo = LevelTwo(proxyAddress); // Note: Use proxyAddress instead of implementation
console2.log("Clara is student:", levelTwo.isStudent(clara));
console2.log("Dan is student:", levelTwo.isStudent(dan));
​
assertTrue(
levelTwo.isStudent(clara),
"Clara graduated with incomplete reviews"
);
assertTrue(
levelTwo.isStudent(dan),
"Dan graduated with incomplete reviews"
);
}
​
function test_POC_GraduateWithNoReviews() public {
// 1. Add teacher
vm.prank(principal);
levelOneProxy.addTeacher(alice);
​
// 2. Enroll student
vm.startPrank(clara);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
// 3. Start session
vm.prank(principal);
levelOneProxy.startSession(70);
​
// 4. Fast forward to session end without any reviews
vm.warp(block.timestamp + 4 weeks);
​
// 5. Deploy LevelTwo and graduate
levelTwoImplementation = new LevelTwo();
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
// 6. Graduate without any reviews
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(address(levelTwoImplementation), data);
​
// Verify student graduated without any reviews
LevelTwo levelTwo = LevelTwo(address(levelTwoImplementation));
assertTrue(
levelTwo.isStudent(clara),
"Clara graduated with no reviews"
);
}
}
​

The vulnerability exists because:

  1. No check for reviewCount[student] == 4 in graduateAndUpgrade

  2. No validation against lastReviewTime to ensure reviews were properly spaced

  3. No enforcement of the review requirement from README: "Students must have gotten all reviews before system upgrade"

Test Output

Ran 1 test for test/MissingReviewValidationTest.t.sol:MissingReviewValidationTest
[PASS] test_POC_GraduateWithoutRequiredReviews() (gas: 1138683)
Logs:
Alice balance change: 3500000000000000000000
Bob balance change: 3500000000000000000000
Principal balance change: 500000000000000000000
Clara is student: true
Dan is student: true
​
Traces:
[1188083] MissingReviewValidationTest::test_POC_GraduateWithoutRequiredReviews()
├─ [0] VM::startPrank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [78232] ERC1967Proxy::fallback(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6])
│ ├─ [73258] LevelOne::addTeacher(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6]) [delegatecall]
│ │ ├─ emit TeacherAdded(: alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [49832] ERC1967Proxy::fallback(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e])
│ ├─ [49358] LevelOne::addTeacher(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e]) [delegatecall]
│ │ ├─ emit TeacherAdded(: bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [152722] ERC1967Proxy::fallback()
│ ├─ [152251] LevelOne::enroll() [delegatecall]
│ │ ├─ [31619] MockUSDC::transferFrom(clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [83022] ERC1967Proxy::fallback()
│ ├─ [82551] LevelOne::enroll() [delegatecall]
│ │ ├─ [9719] MockUSDC::transferFrom(dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd])
│ │ └─ ← [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::startPrank(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6])
│ └─ ← [Return]
├─ [0] VM::warp(604801 [6.048e5])
│ └─ ← [Return]
├─ [28511] ERC1967Proxy::fallback(clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674], true)
│ ├─ [28034] LevelOne::giveReview(clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674], true) [delegatecall]
│ │ ├─ emit ReviewGiven(student: clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674], review: true, studentScore: 100)
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [28511] ERC1967Proxy::fallback(dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd], true)
│ ├─ [28034] LevelOne::giveReview(dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd], true) [delegatecall]
│ │ ├─ emit ReviewGiven(student: dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd], review: true, studentScore: 100)
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::warp(1209601 [1.209e6])
│ └─ ← [Return]
├─ [4611] ERC1967Proxy::fallback(clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674], true)
│ ├─ [4134] LevelOne::giveReview(clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674], true) [delegatecall]
│ │ ├─ emit ReviewGiven(student: clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674], review: true, studentScore: 100)
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [4611] ERC1967Proxy::fallback(dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd], true)
│ ├─ [4134] LevelOne::giveReview(dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd], true) [delegatecall]
│ │ ├─ emit ReviewGiven(student: dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd], review: true, studentScore: 100)
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::warp(2419201 [2.419e6])
│ └─ ← [Return]
├─ [445078] → new LevelTwo@0x2e234DAe75C793f67A35089C9d99245E1C58470b
│ └─ ← [Return] 2223 bytes of code
├─ [2851] MockUSDC::balanceOf(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6]) [staticcall]
│ └─ ← [Return] 0
├─ [2851] MockUSDC::balanceOf(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e]) [staticcall]
│ └─ ← [Return] 0
├─ [2851] MockUSDC::balanceOf(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca]) [staticcall]
│ └─ ← [Return] 0
├─ [0] VM::prank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [78129] ERC1967Proxy::fallback(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca)
│ ├─ [77640] LevelOne::graduateAndUpgrade(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca) [delegatecall]
│ │ ├─ [23750] MockUSDC::transfer(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6], 3500000000000000000000 [3.5e21])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6], value: 3500000000000000000000 [3.5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ [23750] MockUSDC::transfer(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], 3500000000000000000000 [3.5e21])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], 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]
├─ [851] MockUSDC::balanceOf(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6]) [staticcall]
│ └─ ← [Return] 3500000000000000000000 [3.5e21]
├─ [851] MockUSDC::balanceOf(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e]) [staticcall]
│ └─ ← [Return] 3500000000000000000000 [3.5e21]
├─ [851] MockUSDC::balanceOf(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca]) [staticcall]
│ └─ ← [Return] 500000000000000000000 [5e20]
├─ [0] console::log("Alice balance change:", 3500000000000000000000 [3.5e21]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Bob balance change:", 3500000000000000000000 [3.5e21]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Principal balance change:", 500000000000000000000 [5e20]) [staticcall]
│ └─ ← [Stop]
├─ [1378] ERC1967Proxy::fallback(clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674]) [staticcall]
│ ├─ [901] LevelOne::isStudent(clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674]) [delegatecall]
│ │ └─ ← [Return] true
│ └─ ← [Return] true
├─ [0] console::log("Clara is student:", true) [staticcall]
│ └─ ← [Stop]
├─ [1378] ERC1967Proxy::fallback(dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd]) [staticcall]
│ ├─ [901] LevelOne::isStudent(dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd]) [delegatecall]
│ │ └─ ← [Return] true
│ └─ ← [Return] true
├─ [0] console::log("Dan is student:", true) [staticcall]
│ └─ ← [Stop]
├─ [1378] ERC1967Proxy::fallback(clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674]) [staticcall]
│ ├─ [901] LevelOne::isStudent(clara: [0xE55d6ba4bE0A6E0D87c4cA26B7C80779573Dc674]) [delegatecall]
│ │ └─ ← [Return] true
│ └─ ← [Return] true
├─ [0] VM::assertTrue(true, "Clara graduated with incomplete reviews") [staticcall]
│ └─ ← [Return]
├─ [1378] ERC1967Proxy::fallback(dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd]) [staticcall]
│ ├─ [901] LevelOne::isStudent(dan: [0xb72116984E306d834a0ae638688Ef9AF1f7FE2cd]) [delegatecall]
│ │ └─ ← [Return] true
│ └─ ← [Return] true
├─ [0] VM::assertTrue(true, "Dan graduated with incomplete reviews") [staticcall]
│ └─ ← [Return]
└─ ← [Stop]

  1. The test test_POC_GraduateWithoutRequiredReviews demonstrates that students can graduate:

    • With only 2 reviews instead of the required 4

    • The system allows the upgrade to proceed

    • Teachers and principal receive their wages

  2. Even more concerning, test_POC_GraduateWithNoReviews shows that a student can graduate without receiving any reviews at all.

This is a valid vulnerability because:

  1. It violates a core business requirement

  2. It could allow students to graduate without proper evaluation

  3. It undermines the entire review system's purpose

  4. It could lead to unqualified students advancing to the next level

Impact

  • Students can graduate without receiving required evaluations

  • Academic integrity is compromised

  • System requirements are violated

  • Teachers may not be able to properly assess student performance

  • The entire academic evaluation system becomes ineffective

Tools Used

  • Foundry for testing and POC

  • Manual code review

Recommendations

  • Add review count validation:

​
function graduateAndUpgrade(address _levelTwo, bytes memory data) public onlyPrincipal {
// Add validation
for(uint i = 0; i < listOfStudents.length; i++) {
address student = listOfStudents[i];
require(reviewCount[student] == 4, "Student missing required reviews");
}
// Existing logic...
}

  • Add session end validation:

​
require(block.timestamp >= sessionEnd, "Session not yet complete");

  • Add score validation:

​
for(uint i = 0; i < listOfStudents.length; i++) {
address student = listOfStudents[i];
require(studentScore[student] >= cutOffScore, "Student below cutoff score");
}



Updates

Lead Judging Commences

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

cut-off criteria not applied

All students are graduated when the graduation function is called as the cut-off criteria is not applied.

Support

FAQs

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