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

In graduateAndUpgrade, the teacher shares and distribution are not properly calculated, causing the contract to fail every time when trying to distribute payments to the teachers

harisuthan5534798

Description

In the graduateAndUpgrade function, the contract does not divide the teacher share among all teachers. Instead, it gives the full share amount to each teacher. This results in the total teacher payout exceeding the bursary amount, which causes the contract to fail when trying to send payments

Proof of code

Below test case shows how this bug is possible:


function testoverpayment() external{
​
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
address student1 = makeAddr("student1");
address student2 = makeAddr("student2");
address john = makeAddr("third teacher");
usdc.mint(student1, schoolFees);
usdc.mint(student2, schoolFees);
​
vm.startPrank(student1);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
​
vm.startPrank(student2);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
​
​
console2.log("Bursery balance" , levelOneProxy.bursary());
​
​
​
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
levelOneProxy.addTeacher(bob);
levelOneProxy.addTeacher(john);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
vm.stopPrank();
}

Bug Explanation:

  1. Two students enroll in the session, so the bursary amount becomes 1e22.

  2. The principal adds 3 teachers. The Teacher Wage Percentage is set to 35%.

  3. In the current code, each teacher’s share is calculated as:
    1e22 * 35 / 100 = 3.5e21

  4. So each teacher receives 3.5e21. With 3 teachers, the total teacher share becomes 1.05e22, which is more than the bursary balance.

  5. This causes the transaction to fail due to insufficient balance.

  6. In simple terms: the teacher share should be divided among all teachers. But in the current logic, each teacher receives the full share, which is incorrect.

Result

Ran 1 test suite in 1.45s (17.72ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:Encountered 1 failing test in test/LeveOnelAndGraduateTest.t.sol:LevelOneAndGraduateTest[FAIL: ERC20InsufficientBalance(0x90193C961A926261B756D1E5bb255e67ff9498A1, 3000000000000000000000 [3e21], 3500000000000000000000 [3.5e21])] testoverpayment() (gas: 1134550)
Encountered a total of 1 failing tests, 0 tests

Impact

The transaction fails every time graduateAndUpgrade() is called.

  • Teachers are receiving more than they should.

  • The principal’s payout might succeed, but the contract may overpay or lose funds

Tools Used

1) Vs code

2) Manual review

Recommendations :

Update the code to divide the total teacher share among all teachers, instead of giving each one the full 35%

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 eachteacherpay = payPerTeacher / totalTeachers;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
​
_authorizeUpgrade(_levelTwo);
​
for (uint256 n = 0; n < totalTeachers; n++) {
​
-------- usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
​
+++++++ usdc.safeTransfer(listOfTeachers[n], eachteacherpay);
}
​
usdc.safeTransfer(principal, principalPay);
}
Updates

Lead Judging Commences

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

incorrect teacher pay calculation

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

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