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

Critical Upgrade Function Vulnerable to Gas Limit DoS

umarbinkhattab

Description: The LevelOne contract's graduateAndUpgrade() function contains a severe vulnerability that can permanently prevent contract upgrades and payment distributions. The function includes an unbounded loop that iterates through all teachers to transfer tokens, with each transfer consuming significant gas. As the number of teachers grows, this will inevitably exceed block gas limits, rendering the function inoperable and permanently locking the contract in its current implementation.

Code Snippet:

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

Impact: This vulnerability creates a critical flaw in the contract's upgrade mechanism. Once the teacher count exceeds a certain threshold, the upgrade mechanism becomes permanently blocked due to gas limitations. This effectively:

  1. Prevents all future contract upgrades, locking the system in its current implementation

  2. Freezes all teacher and principal payments in the contract

  3. Breaks a core contract feature that's essential for the school's operation and evolution

Detailed Analysis: Each ERC20 transfer operation in the loop requires:

  • Base transaction cost: ~21,000 gas

  • ERC20 transfer cost: ~20,000-50,000 gas (varies by token implementation)

With current Ethereum block gas limits (~30M), this function could handle approximately 600-1000 teachers in optimal conditions. However, the actual limit is likely lower due to other operations in the function and potential complexities in the token transfer implementation.

Proof of Concept:

  1. A school system is deployed and operates successfully with 20 teachers.

  2. Over time, the school grows to 800 teachers.

  3. When the principal attempts to upgrade the contract, the transaction consistently fails due to out of gas errors.

  4. The contract is now stuck in its current implementation, unable to upgrade.

  5. Teacher and principal payments are permanently locked in the contract.

Recommended Mitigation: Implement a withdrawal pattern instead of pushing payments, alongside a batch processing system for upgrades:

// Add state variables
mapping(address => uint256) public pendingPayments;
bool public upgradeAuthorized;
address public upgradeTo;
​
// Split the upgrade and payment functions
function authorizeUpgrade(address _levelTwo) public onlyPrincipal {
if (_levelTwo == address(0)) {
revert HH__ZeroAddress();
}
uint256 payPerTeacher = (bursary * TEACHER_WAGE) / PRECISION;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
// Record pending payments instead of transferring
for (uint256 n = 0; n < listOfTeachers.length; n++) {
pendingPayments[listOfTeachers[n]] = payPerTeacher;
}
pendingPayments[principal] = principalPay;
// Authorize the upgrade
upgradeAuthorized = true;
upgradeTo = _levelTwo;
}
​
// Allow participants to withdraw their funds individually
function withdrawPayment() public {
uint256 amount = pendingPayments[msg.sender];
require(amount > 0, "No payment available");
pendingPayments[msg.sender] = 0;
usdc.safeTransfer(msg.sender, amount);
}
​
// Modify the upgrade function to use authorized upgrade
function graduateAndUpgrade(address, bytes memory) public onlyPrincipal {
require(upgradeAuthorized, "Upgrade not authorized");
_authorizeUpgrade(upgradeTo);
}
​
function _authorizeUpgrade(address newImplementation) internal override onlyPrincipal {}
Updates

Lead Judging Commences

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

inefficient teacher payment system

Due to the use of a push system as regards payment of teacher wages, there is a risk of possible DoS as gas costs increase in direct proportion to size of teachers list.

Support

FAQs

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