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

H-02: Flawed Teacher Wage Distribution Logic Leads to Excessive Payouts

albert

Summary

The graduateAndUpgrade function calculates payPerTeacher as 35% of the total bursary and then pays this amount to each teacher. This results in a total teacher payout of N * 35% * bursary (where N is the number of teachers), far exceeding the intended 35% share and potentially draining the contract or causing reverts due to insufficient funds.

Vulnerability Details

In LevelOne.sol#graduateAndUpgrade:

uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION; // TEACHER_WAGE is 35
// ...
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}

If bursary is 1000 USDC and there are 3 teachers:
payPerTeacher = (1000 * 35) / 100 = 350 USDC.
Each of the 3 teachers receives 350 USDC.
Total payout to teachers = 3 * 350 = 1050 USDC.
The principal also receives 5% (50 USDC).
Total attempted payout = 1050 + 50 = 1100 USDC, which is more than the bursary of 1000 USDC.

Impact

If there is more than one teacher, the total payout to teachers will exceed the intended 35% share. If the contract holds insufficient USDC to cover these inflated payouts, the usdc.safeTransfer calls will revert, blocking wage payments and the upgrade process. This violates the specified payment structure (teachers share 35%) and can lead to a denial of service for wage distribution and system upgrade.

Tools Used

Manual Review, Logical Analysis.

Recommendations

Calculate the total wage pool for all teachers first (35% of bursary). Then, if there are teachers, divide this pool инфекци them.

(The code modification for this is combined with H-03, H-04, and L-03 fixes in the graduateAndUpgrade function shown below H-04.)
Consolidated Code Modification for LevelOne.sol::graduateAndUpgrade (addressing H-02, H-03, H-04, L-03):

// src/LevelOne.sol
// ... (other parts of the contract) ...
​
function graduateAndUpgrade(address _levelTwo, bytes memory dataForLevelTwoInitialize) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
​
// --- START OF MODIFICATION FOR H-04 (Invariant Checks) ---
require(inSession, "HH__NotInSession"); // Added: Ensure session was started
require(block.timestamp >= sessionEnd, "HH__SessionNotEnded");
​
for (uint256 i = 0; i < listOfStudents.length; i++) {
address student = listOfStudents[i];
// Assuming H-05 (reviewCount increment) and M-01 (review limit to 4) are fixed.
require(reviewCount[student] == 4, "HH__StudentReviewIncomplete");
}
// Note: The filtering of students who don't meet cutOffScore is handled
// by LevelTwo's reinitializer using the inherited studentScore and cutOffScore.
// LevelOne must pass its cutOffScore to LevelTwo.
// The `dataForLevelTwoInitialize` should include this.
// --- END OF MODIFICATION FOR H-04 ---
​
uint256 totalTeachers = listOfTeachers.length;
uint256 currentBursary = bursary; // Use a temporary variable for calculations
uint256 principalPay = (currentBursary * PRINCIPAL_WAGE) / PRECISION;
​
// --- START OF MODIFICATION FOR H-02 (Correct Teacher Wage Calculation) ---
uint256 totalTeacherShare = (currentBursary * TEACHER_WAGE) / PRECISION;
uint256 payPerTeacher = 0;
if (totalTeachers > 0) {
payPerTeacher = totalTeacherShare / totalTeachers;
}
// --- END OF MODIFICATION FOR H-02 ---
​
// --- START OF MODIFICATION FOR L-03 (Update bursary state variable) ---
// This reflects that 40% (35% teachers + 5% principal) is paid out.
// The remaining 60% stays in the bursary.
bursary = currentBursary - principalPay - totalTeacherShare;
// --- END OF MODIFICATION FOR L-03 ---
​
// Pay teachers
// If payPerTeacher is 0 (no teachers or zero share), loop won't run or transfers 0.
// Dust from division (if totalTeacherShare % totalTeachers != 0) will remain from totalTeacherShare
// and effectively add to the remaining bursary.
for (uint256 n = 0; n < totalTeachers; n++) {
if (payPerTeacher > 0) { // Avoid 0 value transfers if not needed by token
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
}
​
// Pay principal
if (principalPay > 0) { // Avoid 0 value transfers
usdc.safeTransfer(principal, principalPay);
}
// --- START OF MODIFICATION FOR H-03 (Actual Upgrade Call) ---
// _authorizeUpgrade is an internal function and will be called by _upgradeToAndCallUUPS.
// The `onlyPrincipal` modifier on this function already gates who can call it.
// `dataForLevelTwoInitialize` should be abi.encodeWithSignature("reinitializerFunctionName(types...)", args...)
// For example, for LevelTwo's graduate(): abi.encodeWithSignature("graduate(address,address,uint256)", newPrincipal, usdcAddress, currentCutOffScore)
_upgradeToAndCallUUPS(_levelTwo, dataForLevelTwoInitialize, false);
// --- END OF MODIFICATION FOR H-03 ---
​
emit Graduated(_levelTwo); // Event was present in README example, ensure it's emitted
}
​
// _authorizeUpgrade is an override required by UUPSUpgradeable
// It's called by the UUPS mechanism during an upgrade, not directly by us before calling _upgradeToAndCallUUPS.
// The onlyPrincipal modifier on graduateAndUpgrade already ensures only principal can initiate.
function _authorizeUpgrade(address newImplementation) internal override onlyPrincipal {}
​
// ... (other parts of the contract) ...
Updates

Lead Judging Commences

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