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

Missing division by totalTeachers causes 35% bursary to be sent to every teacher causing fund mismanagement and DOS.

vanshikag

Summary

In calculating payPerTeacher, there is no division by the number of teachers, which means each teacher gets 35% of the bursary. This breaks a contract invariant and if there are 3 or more teachers, it will cause DOS of the upgrade functionality.

Vulnerability Details

Wages for teachers are supposed to be 35% of bursary total. In calculation of these fees in LevelOne.graduateAndUpgrade(), the division by totalTeachers is missing. This leads to 35% of the bursary being sent to every teacher, instead of being the total transferred value. The relevant part of the code is below:

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
​
uint256 totalTeachers = listOfTeachers.length;
// sending 35% of bursary PER TEACHER instead of splitting it.
// should divide by totalTeachers here.
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);
}

Impact

No impact if there is only 1 teacher. 70% of the funds used instead of 35% if there are two teachers. 105% of the bursary funds would be required for 3 teachers. Having >= 3 teachers causes DOS in the function and the contract cannot be upgraded.

Proof Of Concept

Add the following line to LevelOneAndGraduateTeat.test_confirm_can_graduate() and run with forge test --mt test_confirm_can_graduate -vvv.

function test_confirm_can_graduate() public schoolInSession {
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
​
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
​
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
​
// console2.log(levelTwoProxy.bursary()); // this is a different bug.
// expected = 60% = 2e22. actual = 25% = 7.5e21
+ console2.log(usdc.balanceOf(address(levelTwoProxy))); //@audit
console2.log(levelTwoProxy.getTotalStudents());
}

Expected Result:

[PASS] test_confirm_can_graduate() (gas: 1174292)
Logs:
7500000000000000000000
6
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.53ms (399.25µs CPU time)
​
Ran 1 test suite in 5.54ms (1.53ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

If you add another address to _teachersAdded() in the test file (teachers = 3), and rerun the test, you get the following result:

Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 6.33ms (1.10ms CPU time)
​
Ran 1 test suite in 1.28s (6.33ms 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, 9000000000000000000000 [9e21], 10500000000000000000000 [1.05e22])] test_confirm_can_graduate() (gas: 1340629)
​
Encountered a total of 1 failing tests, 0 tests succeeded

Tools Used

Manual Review

Recommendation

Divide payPerTeacher by totalTeachers.

Updates

Lead Judging Commences

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