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

Incorrect Teacher Wage Calculation in `LevelOne::graduateAndUpgrade` Leads to Contract Fund Drain

geekybot

Summary

The LevelOne::graduateAndUpgrade function in the LevelOne contract incorrectly calculates teacher wages during the graduation process. While a total of 35% of the bursary is intended to be distributed among all teachers, the current implementation allocates the entire 35% to each teacher, resulting in a potential fund drain.

Vulnerability Details

In the graduateAndUpgrade function, the teacher wage calculation is performed as follows:

uint256 totalTeachers = listOfTeachers.length;
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;

The issue is that payPerTeacher calculates 35% of the total bursary (TEACHER_WAGE = 35, PRECISION = 100), but doesn't divide this amount by the number of teachers. Later, this amount is transferred to each teacher in a loop:

for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}

Proof Of Code

function test_teacher_wage_calculation_bug() public schoolInSession {
// Setup
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
// Record initial balances
uint256 initialBalance = usdc.balanceOf(address(levelOneProxy));
uint256 initialAliceBalance = usdc.balanceOf(alice);
uint256 initialBobBalance = usdc.balanceOf(bob);
// Log values for clarity
console2.log("Initial contract balance:", initialBalance);
console2.log("Initial Alice balance:", initialAliceBalance);
console2.log("Initial Bob balance:", initialBobBalance);
console2.log("Total teachers:", levelOneProxy.getTotalTeachers());
console2.log("Bursary amount:", levelOneProxy.bursary());
// Calculate expected amounts
uint256 totalTeachers = levelOneProxy.getTotalTeachers();
uint256 bursary = levelOneProxy.bursary();
// Expected: Each teacher should get (bursary * TEACHER_WAGE) / (PRECISION * totalTeachers)
uint256 expectedTeacherWage = (bursary * levelOneProxy.TEACHER_WAGE()) /
(levelOneProxy.PRECISION() * totalTeachers);
// Actual: Each teacher gets (bursary * TEACHER_WAGE) / PRECISION
uint256 actualTeacherWage = (bursary * levelOneProxy.TEACHER_WAGE()) /
levelOneProxy.PRECISION();
console2.log("Expected wage per teacher:", expectedTeacherWage);
console2.log("Actual wage per teacher:", actualTeacherWage);
console2.log("Difference per teacher:", actualTeacherWage - expectedTeacherWage);
// Perform graduation
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
// Check balances after graduation
uint256 finalAliceBalance = usdc.balanceOf(alice);
uint256 finalBobBalance = usdc.balanceOf(bob);
uint256 finalContractBalance = usdc.balanceOf(address(levelOneProxy));
console2.log("Alice received:", finalAliceBalance - initialAliceBalance);
console2.log("Bob received:", finalBobBalance - initialBobBalance);
console2.log("Total paid to teachers:",
(finalAliceBalance - initialAliceBalance) + (finalBobBalance - initialBobBalance));
console2.log("Final contract balance:", finalContractBalance);
// Verify the bug: each teacher received the full teacher allocation instead of a share
assertEq(finalAliceBalance - initialAliceBalance, actualTeacherWage);
assertEq(finalBobBalance - initialBobBalance, actualTeacherWage);
// Demonstrate issue: Total paid is more than intended
uint256 totalTeacherPayout = (finalAliceBalance - initialAliceBalance) +
(finalBobBalance - initialBobBalance);
uint256 expectedTotalPayout = (bursary * levelOneProxy.TEACHER_WAGE()) / levelOneProxy.PRECISION();
console2.log("Expected total teacher payout:", expectedTotalPayout);
console2.log("Actual total teacher payout:", totalTeacherPayout);
// Assert that total teacher payout is greater than expected (e.g., 70% instead of 35%)
assertEq(totalTeacherPayout, actualTeacherWage * totalTeachers);
assertTrue(totalTeacherPayout > expectedTotalPayout);
assertEq(totalTeacherPayout, expectedTotalPayout * totalTeachers);
}

Impact

  • If there are multiple teachers (which is the intended use case), the contract will transfer significantly more funds than intended.

  • For example, with 5 teachers and a bursary of 100,000 USDC:

    Expected: Each teacher receives 7,000 USDC (35,000 USDC total, 35% of bursary)
    Actual: Each teacher receives 35,000 USDC (175,000 USDC total, 175% of bursary)

  • This could completely drain the contract funds and make graduation impossible if sufficient funds aren't available.

  • The error becomes more severe as more teachers are added to the system.

Tools Used

  • Manual code review

  • Mathematical analysis

Recommendations

Modify the graduateAndUpgrade function to correctly calculate the payment per teacher by dividing the total teacher allocation by the number of teachers:

// Calculate total amount for teachers (35% of bursary)
uint256 totalTeacherPay = (bursary * TEACHER_WAGE) / PRECISION;
​
// Divide by number of teachers to get per-teacher amount
uint256 payPerTeacher = totalTeacherPay / totalTeachers;

Or more concisely:

uint256 payPerTeacher = (bursary * TEACHER_WAGE) / (PRECISION * totalTeachers);

Additionally, consider adding a check to prevent division by zero in case there are no teachers:

uint256 payPerTeacher = totalTeachers > 0 ?
(bursary * TEACHER_WAGE) / (PRECISION * totalTeachers) : 0;
Updates

Lead Judging Commences

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

incorrect teacher pay calculation

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

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