Hawk High
First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

Division by Zero Risk

kweks

Summary

The graduateAndUpgrade function attempts to distribute teacher payments by dividing teachersShare by totalTeachers. However, if listOfTeachers is empty, this results in a division-by-zero error that reverts the entire transaction, halting upgrades and locking funds indefinitely.

Vulnerability Details

If listOfTeachers.length == 0, this code becomes dangerous:

uint256 totalTeachers = listOfTeachers.length;
uint256 payPerTeacher = teachersShare / totalTeachers; // ❌ division by zero

This reverts the transaction entirely, preventing:

  • Payment to the principal

  • Upgrade to LevelTwo

  • Graduation of students

  • Bursary adjustment

Exploit Scenario

  1. listOfTeachers = [] (no teachers).

  2. Principal calls graduateAndUpgrade().

  3. totalTeachers = 0 → Division by zero.

  4. Entire upgrade fails and bursary remains locked.

  5. No way to proceed without a contract upgrade or new teacher registration.

Impact

  • Denial of Service: Contract is rendered unusable for upgrades when no teachers are present.

  • Fund Locking: USDC in bursary cannot be transferred or upgraded.

  • Protocol Freeze: Prevents all graduating actions for an entire session.

Tools Used

Manual review

Recommendations

✅ Fix Implementation

Add a check for totalTeachers > 0 before computing payPerTeacher:

function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
// ... session and review checks ...
​
uint256 totalTeachers = listOfTeachers.length;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
uint256 teachersShare = (bursary * TEACHER_WAGE) / PRECISION;
​
// Deduct funds first (effects before interactions)
bursary -= (principalPay + teachersShare); // Safe even if no teachers
​
usdc.safeTransfer(principal, principalPay);
​
// ========== CRITICAL FIX ========== //
if (totalTeachers > 0) {
uint256 payPerTeacher = teachersShare / totalTeachers;
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
} else {
emit TeacherPaymentSkipped(); // Optional transparency
}
// ================================== //
​
// ... continue with upgrade ...
}
​

Key Fixes

  • Edge Case Handling: Skips teacher payment logic when none exist.

  • State Safety: Ensures bursary is still reduced, complying with financial invariants.

  • Smooth Upgrades: Avoids transaction reverts that would block core functionality.

Why This Matters

  • Protocol Continuity: Keeps upgrades running even in rare or testing scenarios with no teachers.

  • Invariant Integrity: 60% bursary is correctly maintained post-payment logic.

  • Robustness: Handles all input conditions gracefully without locking user funds.

Optional Enhancements

  • Add a require(totalTeachers > 0) early in the session lifecycle (if teachers must always be present).

  • Emit TeacherPaymentSkipped event for auditability and transparency.

Updates

Lead Judging Commences

yeahchibyke Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.