Hawk High

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

Violation of Invariant: Residual Bursary Not Carried Over to New Implementation

Summary

The LevelOne contract fails to manage the remaining 60% of the bursary (school fees collected) as per the project's defined invariants during the graduateAndUpgrade process. Specifically, these funds are not transferred or made accessible to the new contract implementation (LevelTwo) after the principal and teachers are paid. This results in the funds becoming effectively stranded within the LevelOne contract, directly violating the invariant that "remaining 60% should reflect in the bursary after upgrade."

Vulnerability Details

The project's invariants explicitly state: "remaining 60% should reflect in the bursary after upgrade." This implies that after the principal (5%) and teachers (35%) receive their wages from the bursary, the substantial remainder (60%) should be available to or form part of the bursary of the newly upgraded system.

However, the LevelOne.graduateAndUpgrade(address _levelTwo, bytes memory) function operates as follows:

  1. It calculates payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION.

  2. It calculates principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION.

  3. It calls _authorizeUpgrade(_levelTwo) to set up the UUPS upgrade.

  4. It transfers payPerTeacher to each teacher.

  5. It transfers principalPay to the principal.

At this point, the function concludes. There is no mechanism or code within graduateAndUpgrade to:

  • Transfer the remaining USDC tokens (representing 60% of the original bursary value) from the LevelOne contract's balance to the _levelTwo contract address.

  • Update any state in _levelTwo to reflect this amount.

The bursary state variable in LevelOne itself is not reset or decremented beyond the implicit value it held before payments. The actual USDC tokens corresponding to this 60% remain held by the LevelOne contract address. The LevelTwo contract, as provided, also has no function to pull or claim these funds from LevelOne post-upgrade.

This directly contravenes the specified operational requirement for the residual bursary.

Impact

The failure to manage and transfer the residual 60% of the bursary has several significant negative impacts:

  1. Direct Financial Loss:

    • 60% of all school fees collected and held in the bursary at the time of upgrade become inaccessible and effectively lost to the Hawk High school entity. These funds cannot be used for future operational costs, reinvestment, development of LevelTwo (or subsequent levels), or to benefit future student cohorts as might be intended.

  2. Breach of Core Business Logic and Invariants:

    • The fundamental financial model and a key invariant of the school's operation are violated. This indicates a flaw in the contract's adherence to its specified design.

  3. Operational Disruption for LevelTwo:

    • If the LevelTwo contract (or the school's operational model) anticipates having access to these residual funds to form its own initial bursary or to cover its operational expenses, it will start with a significant, un-catered-for deficit.

  4. Erosion of Trust and Accountability:

    • Mishandling or "losing" a significant portion of collected fees can severely damage the trust of users (students, parents, stakeholders) in the platform's financial management and its ability to operate according to its own stated rules.

  5. Potential for Unintended Accumulation:

    • If the LevelOne contract address somehow remains active or accessible after the upgrade (though UUPS typically redirects calls), these funds would just sit there, potentially accumulating across multiple "sessions" if the upgrade process was flawed in other ways, further complicating accounting.

Tools Used

manual review

Recommendations

To rectify the mishandling of the residual 60% of the bursary and ensure compliance with the project invariant, the following changes and considerations are recommended:

  1. Modify LevelOne.graduateAndUpgrade to Transfer Residual Funds:

    • Calculate Residual Amount: After calculating principalPay and the total amount to be paid to all teachers, determine the remainingBursary.

      // Inside graduateAndUpgrade, after principal and teacher shares are calculated
      uint256 totalPaidToTeachers = 0;
      if (listOfTeachers.length > 0) { // Avoid division by zero if no teachers
      // payPerTeacher was already calculated as (bursary * TEACHER_WAGE) / PRECISION
      // This calculation might be better done as:
      // uint256 totalTeacherShare = (bursary * TEACHER_WAGE) / PRECISION;
      // uint256 payPerTeacher = totalTeacherShare / listOfTeachers.length; // if distributing equally
      // For simplicity here, assuming payPerTeacher is the amount each gets.
      // The original code implies payPerTeacher is the individual share.
      totalPaidToTeachers = payPerTeacher * listOfTeachers.length;
      }
      uint256 fundsDisbursed = principalPay + totalPaidToTeachers;
      uint256 currentBursaryBalance = usdc.balanceOf(address(this)); // More robust than relying on the bursary state variable if external deposits are possible
      // It's crucial to ensure `bursary` accurately reflects only the current session's fees.
      // If `bursary` is cumulative, this logic needs to be tied to a session-specific bursary.
      // Assuming `bursary` state variable is the correct source for calculation:
      if (fundsDisbursed > bursary) {
      // This case should ideally not happen if bursary management is correct.
      // Handle error or set remainingBursary to 0.
      // For now, assume bursary >= fundsDisbursed
      }
      uint256 remainingBursaryToTransfer = bursary - fundsDisbursed;
      // A more robust way if other contract interactions modify balance:
      // uint256 remainingBursaryToTransfer = currentBursaryBalance - fundsDisbursed;

      content_copydownload

      Use code with caution.Solidity

    • Transfer to _levelTwo: Before the function concludes, transfer these remainingBursaryToTransfer funds to the _levelTwo contract address.

      // ... (payments to teachers and principal) ...
      if (remainingBursaryToTransfer > 0) {
      usdc.safeTransfer(_levelTwo, remainingBursaryToTransfer);
      }

      content_copydownload

      Use code with caution.Solidity

    • Update LevelOne.bursary State (Crucial for Session Logic): After all transfers (principal, teachers, and residual to _levelTwo), the bursary variable in LevelOne should be reset to 0 or appropriately reduced to reflect that the funds for that session have been fully processed. This is critical if LevelOne persists or to prevent miscalculation in unlikely re-entry scenarios.

      // At the very end of graduateAndUpgrade
      bursary = 0; // Or bursary -= (fundsDisbursed + remainingBursaryToTransfer);

      content_copydownload

      Use code with caution.Solidity

  2. Ensure LevelTwo Can Receive and Manage Funds:

    • The LevelTwo contract (and its initialize or initializeV2 function) should be designed to be aware that it might start with an initial USDC balance if this transfer occurs.

    • Its internal bursary variable should be set or incremented based on the USDC balance it holds upon initialization/upgrade, or it should account for these funds as intended by the school's operational model.

      // Example within LevelTwo's initializer
      // function initializeV2(...) public reinitializer(2) { // Or however it's structured
      // ...
      // bursary = usdc.balanceOf(address(this));
      // ...
      // }

      content_copydownload

      Use code with caution.Solidity

  3. Emit an Event for Transparency:

    • Add an event in LevelOne to log the transfer of residual bursary funds to the new contract.

      // In LevelOne
      event ResidualBursaryTransferred(address indexed to, uint256 amount);
      // In graduateAndUpgrade, after the transfer
      if (remainingBursaryToTransfer > 0) {
      usdc.safeTransfer(_levelTwo, remainingBursaryToTransfer);
      emit ResidualBursaryTransferred(_levelTwo, remainingBursaryToTransfer);
      }

      content_copydownload

      Use code with caution.Solidity

  4. Address Overall Bursary Management (Session-Specific):

    • Strongly Recommended: The underlying issue that bursary in LevelOne is a cumulative figure across all sessions needs to be addressed. Implement a sessionBursary that is reset at the start of each new session (or after graduation). All calculations (enrollment fees, principal wage, teacher wages, residual) should be based on this sessionBursary. This will make the 60% residual calculation accurate for the current graduating session.

    • If this is not done, the "60% residual" will be 60% of all funds ever collected and not yet spent, which may not be the intended logic.

  5. Thorough Testing:

    • Test the upgrade process extensively, specifically verifying:

      • Correct calculation of principal, teacher, and residual shares.

      • Successful transfer of USDC to all parties, including LevelTwo.

      • The balance of LevelOne after upgrade (should be 0 or significantly reduced for USDC).

      • The balance and bursary state of LevelTwo immediately after upgrade.

      • Edge cases: no teachers, zero bursary, etc.

By implementing these recommendations, the contract will align with the specified invariant, ensuring that the residual 60% of the (session-specific) bursary is properly handled and made available to the subsequent contract version, preventing financial loss and maintaining the integrity of the school's financial operations.


Updates

Lead Judging Commences

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

bursary not updated

The bursary is not updated after wages have been paid in `graduateAndUpgrade()` function

Support

FAQs

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