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

Incorrect Teacher Wage Distribution in LevelOne Contract (Business Logic Vulnerability)

teheelaa

Summary

In the LevelOne contract, the graduateAndUpgrade function incorrectly distributes teacher wages by giving each teacher the full 35% of the bursary instead of splitting this percentage among all teachers. This results in the contract paying out multiples of the intended teacher wage allocation.

The vulnerability leads to significant financial loss as teachers receive more compensation than intended, depleting the contract's funds faster than designed.

This issue occurs every time the graduateAndUpgrade function is called, making it highly likely to be exploited.

Vulnerability Details

The vulnerability exists in the graduateAndUpgrade function of the LevelOne contract:

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; // @audit - incorrect calculation
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
​
_authorizeUpgrade(_levelTwo);
​
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher); // @audit - each teacher gets full 35%
}
​
usdc.safeTransfer(principal, principalPay);
}
​

The issue arises because:

  1. TEACHER_WAGE is set to 35 (35%)

  2. payPerTeacher is calculated as (bursary * 35) / 100

  3. This full 35% is then paid to each teacher in the loop

Here's a Proof of Concept demonstrating the vulnerability:

// SPDX-License-Identifier: MIT
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 TeacherWageVulnerabilityTest is Test {
DeployLevelOne deployBot;
LevelOne levelOneProxy;
MockUSDC usdc;
address principal;
uint256 schoolFees;
​
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address student = makeAddr("student");
​
function setUp() public {
deployBot = new DeployLevelOne();
address proxyAddress = deployBot.deployLevelOne();
levelOneProxy = LevelOne(proxyAddress);
usdc = deployBot.getUSDC();
principal = deployBot.principal();
schoolFees = deployBot.getSchoolFees();
​
// Mint USDC for student
usdc.mint(student, schoolFees);
}
​
function testTeacherWageVulnerability() public {
// 1. Add two teachers
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
levelOneProxy.addTeacher(bob);
vm.stopPrank();
​
// 2. Enroll student and start session
vm.startPrank(student);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
vm.prank(principal);
levelOneProxy.startSession(70);
​
// Record initial balances
uint256 initialContractBalance = usdc.balanceOf(address(levelOneProxy));
uint256 initialAliceBalance = usdc.balanceOf(alice);
uint256 initialBobBalance = usdc.balanceOf(bob);
​
// 3. Graduate and upgrade
LevelTwo levelTwoImplementation = new LevelTwo();
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(address(levelTwoImplementation), data);
​
// 4. Verify incorrect payment
uint256 expectedTeacherShare = (initialContractBalance * levelOneProxy.TEACHER_WAGE()) / levelOneProxy.PRECISION() / 2; // Should be divided by number of teachers
uint256 actualAlicePayout = usdc.balanceOf(alice) - initialAliceBalance;
uint256 actualBobPayout = usdc.balanceOf(bob) - initialBobBalance;
​
console2.log("Expected teacher share:", expectedTeacherShare);
console2.log("Actual Alice payout:", actualAlicePayout);
console2.log("Actual Bob payout:", actualBobPayout);
​
assertTrue(
actualAlicePayout > expectedTeacherShare,
"Alice received more than intended share"
);
assertTrue(
actualBobPayout > expectedTeacherShare,
"Bob received more than intended share"
);
​
// Verify total payout exceeds intended allocation
uint256 totalTeacherPayout = actualAlicePayout + actualBobPayout;
uint256 intendedTeacherAllocation = (initialContractBalance * levelOneProxy.TEACHER_WAGE()) / levelOneProxy.PRECISION();
assertTrue(
totalTeacherPayout > intendedTeacherAllocation,
"Total teacher payout exceeds intended allocation"
);
}
}

Test Output

Ran 1 test for test/TeacherWageVulnerabilityTest.t.sol:TeacherWageVulnerabilityTest
[PASS] testTeacherWageVulnerability() (gas: 948773)
Logs:
Expected teacher share: 875000000000000000000
Actual Alice payout: 1750000000000000000000
Actual Bob payout: 1750000000000000000000
​
Traces:
[973473] TeacherWageVulnerabilityTest::testTeacherWageVulnerability()
├─ [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(student: [0x29C333Ff20d076399617Aa0C800925CE947D82B8])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: student: [0x29C333Ff20d076399617Aa0C800925CE947D82B8], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [152722] ERC1967Proxy::fallback()
│ ├─ [152251] 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]
├─ [851] MockUSDC::balanceOf(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1]) [staticcall]
│ └─ ← [Return] 5000000000000000000000 [5e21]
├─ [2851] MockUSDC::balanceOf(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6]) [staticcall]
│ └─ ← [Return] 0
├─ [2851] MockUSDC::balanceOf(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e]) [staticcall]
│ └─ ← [Return] 0
├─ [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(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6], 1750000000000000000000 [1.75e21])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6], value: 1750000000000000000000 [1.75e21])
│ │ │ └─ ← [Return] true
│ │ ├─ [23750] MockUSDC::transfer(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], 1750000000000000000000 [1.75e21])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e], 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]
├─ [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(alice: [0x328809Bc894f92807417D2dAD6b7C998c1aFdac6]) [staticcall]
│ └─ ← [Return] 1750000000000000000000 [1.75e21]
├─ [851] MockUSDC::balanceOf(bob: [0x1D96F2f6BeF1202E4Ce1Ff6Dad0c2CB002861d3e]) [staticcall]
│ └─ ← [Return] 1750000000000000000000 [1.75e21]
├─ [0] console::log("Expected teacher share:", 875000000000000000000 [8.75e20]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Actual Alice payout:", 1750000000000000000000 [1.75e21]) [staticcall]
│ └─ ← [Stop]
├─ [0] console::log("Actual Bob payout:", 1750000000000000000000 [1.75e21]) [staticcall]
│ └─ ← [Stop]
├─ [0] VM::assertTrue(true, "Alice received more than intended share") [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertTrue(true, "Bob received more than intended share") [staticcall]
│ └─ ← [Return]
├─ [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
├─ [0] VM::assertTrue(true, "Total teacher payout exceeds intended allocation") [staticcall]
│ └─ ← [Return]
└─ ← [Stop]

Impact

  1. Financial Loss: The contract pays out multiple times the intended teacher wage allocation

  2. Fund Depletion: The remaining 60% of funds meant to stay in the bursary is significantly reduced

  3. Economic Imbalance: Teachers receive more compensation than designed, breaking the intended tokenomics

For example, with 2 teachers and a bursary of 1000 USDC:

  • Intended: Each teacher should receive 175 USDC (35% split between 2)

  • Actual: Each teacher receives 350 USDC (35% each), totaling 700 USDC (70%)

Tools Used

  • Manual code review

  • Foundry testing framework

Recommendations

Modify the graduateAndUpgrade function to correctly split the teacher wage among all teachers:

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

Additionally:

  1. Add a check to prevent division by zero when there are no teachers

  2. Consider adding events to track wage distributions

  3. Add documentation clearly stating how teacher wages are calculated and distributed


Updates

Lead Judging Commences

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

incorrect teacher pay calculation

`payPerTeacher` in `graduateAndUpgrade()` is incorrectly calculated.

Support

FAQs

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