Hawk High

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

Incorrect payroll formula traps funds or can drain the contract (Root Cause: teachers × 35 + 5 not validated

The graduateAndUpgrade() function computes payroll as follows:

uint256 teacherWage = TEACHER_WAGE; // 35 (→ 35 %)
uint256 principalWage= PRINCIPAL_WAGE; // 5 (→ 5 %)
for (uint256 i; i < listOfTeachers.length; ++i) {
usdc.transfer(listOfTeachers[i],
(bursary * teacherWage) / PRECISION);
}
usdc.transfer(principal,
(bursary * principalWage) / PRECISION);
  • Each teacher is paid 35% of the entire bursary (instead of that 35% being split among them).

  • The principal is paid an additional 5%.

  • The code does not check that (number of teachers × 35) + 5 ≤ 100.

With 2 teachers, 75% is paid and 25% remains “orphaned.”
With 3 or more teachers, the total exceeds 100% and the second transfer reverts for lack of funds, blocking graduation entirely.

Impact:

  • Trapped funds: at least 25% of the bursary is always left in the contract (expected business logic would leave 60%); those USDC cannot be withdrawn.

  • Financial DoS risk: if the teacher count pushes payments over 100%, the function always reverts—no one can graduate or upgrade the system.

  • Economic misalignment: students pay for a service that gets locked in the contract.

Proof of Concept:

function test_leftoverBursary60Percent() public {
// ───── pre-condiciones ─────────────────────────────────────────
_teachersAdded(); // añade 2 profesores
_studentsEnrolled(); // 6 alumnos → bursary inicial = 6 * schoolFees
vm.prank(principal);
levelOneProxy.startSession(70);
uint256 bursaryBefore = usdc.balanceOf(address(levelOneProxy)); // 30e21
// ───── se ejecuta graduateAndUpgrade (paga nómina) ────────────
LevelTwo impl = new LevelTwo();
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(address(impl), data);
// ───── calculo esperado del sobrante buggy ────────────────────
uint256 teachers = levelOneProxy.getTotalTeachers(); // 2
uint256 teacherWage = levelOneProxy.TEACHER_WAGE(); // 35
uint256 principalWage= levelOneProxy.PRINCIPAL_WAGE(); // 5
uint256 precision = levelOneProxy.PRECISION(); // 100
uint256 expected = bursaryBefore
- ((bursaryBefore * teacherWage) / precision) * teachers // 35 % * 2
- (bursaryBefore * principalWage) / precision; // 5 %
// ───── comprobación ───────────────────────────────────────────
assertEq(
usdc.balanceOf(address(levelOneProxy)),
expected,
"El bursary sobrante no coincide con el calculo buggy"
);
}

output

❯ forge test --mt test_leftoverBursary60Percent -vvvv
Warning: This is a nightly build of Foundry. It is recommended to use the latest stable version. Visit https://book.getfoundry.sh/announcements for more information.
To mute this warning set `FOUNDRY_DISABLE_NIGHTLY_WARNING` in your environment.
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/LeveOnelAndGraduateTest.t.sol:LevelOneAndGraduateTest
[PASS] test_leftoverBursary60Percent() (gas: 1377488)
Traces:
[1525688] LevelOneAndGraduateTest::test_leftoverBursary60Percent()
├─ [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::startPrank(third_student: [0xF238496034cA4D476743d590ff3A66def743F9be])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: third_student: [0xF238496034cA4D476743d590ff3A66def743F9be], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [83022] ERC1967Proxy::fallback()
│ ├─ [82551] LevelOne::enroll() [delegatecall]
│ │ ├─ [9719] MockUSDC::transferFrom(third_student: [0xF238496034cA4D476743d590ff3A66def743F9be], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: third_student: [0xF238496034cA4D476743d590ff3A66def743F9be], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: third_student: [0xF238496034cA4D476743d590ff3A66def743F9be])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(fourth_student: [0x8E4a21e39349dBb0178CC57ABF60EF8c78ea2680])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: fourth_student: [0x8E4a21e39349dBb0178CC57ABF60EF8c78ea2680], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [83022] ERC1967Proxy::fallback()
│ ├─ [82551] LevelOne::enroll() [delegatecall]
│ │ ├─ [9719] MockUSDC::transferFrom(fourth_student: [0x8E4a21e39349dBb0178CC57ABF60EF8c78ea2680], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: fourth_student: [0x8E4a21e39349dBb0178CC57ABF60EF8c78ea2680], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: fourth_student: [0x8E4a21e39349dBb0178CC57ABF60EF8c78ea2680])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(fifth_student: [0xA1bC13cEF3390113AcA9450673182D3BEC1ce6Dd])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: fifth_student: [0xA1bC13cEF3390113AcA9450673182D3BEC1ce6Dd], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [83022] ERC1967Proxy::fallback()
│ ├─ [82551] LevelOne::enroll() [delegatecall]
│ │ ├─ [9719] MockUSDC::transferFrom(fifth_student: [0xA1bC13cEF3390113AcA9450673182D3BEC1ce6Dd], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: fifth_student: [0xA1bC13cEF3390113AcA9450673182D3BEC1ce6Dd], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: fifth_student: [0xA1bC13cEF3390113AcA9450673182D3BEC1ce6Dd])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(six_student: [0x983Cec4DF373E6f3809b4483fEBf6C9469B0769b])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: six_student: [0x983Cec4DF373E6f3809b4483fEBf6C9469B0769b], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [83022] ERC1967Proxy::fallback()
│ ├─ [82551] LevelOne::enroll() [delegatecall]
│ │ ├─ [9719] MockUSDC::transferFrom(six_student: [0x983Cec4DF373E6f3809b4483fEBf6C9469B0769b], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: six_student: [0x983Cec4DF373E6f3809b4483fEBf6C9469B0769b], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: six_student: [0x983Cec4DF373E6f3809b4483fEBf6C9469B0769b])
│ │ └─ ← [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] 30000000000000000000000 [3e22]
├─ [445078] → new LevelTwo@0x2e234DAe75C793f67A35089C9d99245E1C58470b
│ └─ ← [Return] 2223 bytes of code
├─ [0] VM::prank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [84129] ERC1967Proxy::fallback(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca)
│ ├─ [83640] LevelOne::graduateAndUpgrade(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca) [delegatecall]
│ │ ├─ [25750] MockUSDC::transfer(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3], 10500000000000000000000 [1.05e22])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3], value: 10500000000000000000000 [1.05e22])
│ │ │ └─ ← [Return] true
│ │ ├─ [25750] MockUSDC::transfer(second_teacher: [0xb4c265c1f1d07474E3715F65724E8fa9d662BF0e], 10500000000000000000000 [1.05e22])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: second_teacher: [0xb4c265c1f1d07474E3715F65724E8fa9d662BF0e], value: 10500000000000000000000 [1.05e22])
│ │ │ └─ ← [Return] true
│ │ ├─ [25750] MockUSDC::transfer(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca], 1500000000000000000000 [1.5e21])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca], value: 1500000000000000000000 [1.5e21])
│ │ │ └─ ← [Return] true
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [1004] ERC1967Proxy::fallback() [staticcall]
│ ├─ [530] LevelOne::getTotalTeachers() [delegatecall]
│ │ └─ ← [Return] 2
│ └─ ← [Return] 2
├─ [910] ERC1967Proxy::fallback() [staticcall]
│ ├─ [436] LevelOne::TEACHER_WAGE() [delegatecall]
│ │ └─ ← [Return] 35
│ └─ ← [Return] 35
├─ [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
├─ [851] MockUSDC::balanceOf(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1]) [staticcall]
│ └─ ← [Return] 7500000000000000000000 [7.5e21]
├─ [0] VM::assertEq(7500000000000000000000 [7.5e21], 7500000000000000000000 [7.5e21], "El bursary sobrante no coincide con el calculo buggy") [staticcall]
│ └─ ← [Return]
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.16ms (967.92µs CPU time)
Ran 1 test suite in 768.48ms (3.16ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation:

  • Redefine the payroll formula.

uint256 totalShare = (teacherWage * teachers) + principalWage;
require(totalShare <= PRECISION, "Payroll shares exceed 100%");
uint256 perTeacher = (bursary * teacherWage) / PRECISION / teachers;
for (...) { usdc.transfer(t, perTeacher); }
  • Or set TEACHER_WAGE as the total teachers’ share (e.g., 35% split among all), document this change, and rename it to TEACHERS_SHARE.

  • Add a function allowing the principal to withdraw any leftover bursary after graduation to prevent trapped funds.

Updates

Lead Judging Commences

yeahchibyke Lead Judge about 2 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.