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

Bursary is not updated correctly

bavoen

Summary

The bursary of the school is not updated correctly when the contract is upgraded. During the graduation and upgrade process the principal and teachers are paid their share of the bursary, however the bursary value is not updated to reflect this payment. This means that the bursary value will not reflect the correct amount of money left in the contract after the payments have been made.

Vulnerability Details

The vulnerability is located in the graduateAndUpgrade function of the LevelOne contract.

function graduateAndUpgrade(address _levelTwo, bytes memory data) 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);
}

We can see that the payments are made to the teachers and principal but the bursary value is not updated accordingly.

The following test will validate that the bursary is equal to the coins in the contract after a graduation and upgrade:

function test_validate_bursary_after_upgrade() public schoolInSession {
uint256 bursaryBefore = levelOneProxy.bursary();
assertEq(bursaryBefore, usdc.balanceOf(address(levelOneProxy)));
​
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
​
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
​
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
​
assertEq(levelTwoProxy.bursary(), usdc.balanceOf(address(levelTwoProxy)));
}

As we can see the bursary value stays the same while the balance of the contract is lowered.

Impact

Since the bursary value is not updated correctly on upgrade the next time the payments are calculated it will be based on the old bursary value. This will lead to the teachers and principal being overpaid until all the coins are spent. After this happens the contract will be locked since the payout will be reverted.

Tools Used

Manually reviewed the code and the documentation.

Recommendations

The bursary value should be updated when making payments:

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);
+ bursary -= payPerTeacher;
}
​
usdc.safeTransfer(principal, principalPay);
+ bursary -= principalPay;
}

Note that the payPerTeacher is calculated incorectly but this will be part of an other report. This recomendation makes sure the bursary value and the holdings of the contract stay in sync.

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.