Summary
During the LevelOne::graduateAndUpgrade function, the bursary calculation is incorrect. The principal and teacher balances are updated correctly, but the bursary is not and does not account for the contract balance. This can lead to discrepancies in the expected and actual balances after the upgrade.
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
The actual bursary amount may not be accurately reflected in the contract balance, leading to potential financial discrepancies and loss of funds.
Proof of Concept
Let's assume the following values:
Old bursary: 30000000000000000000000
Principal USDC balance: 0
Teacher USDC balance: 0
Contract balance: 30000000000000000000000
After the graduateAndUpgrade function is called, the new balances are:
New bursary: 30000000000000000000000
New Principal USDC balance: 1500000000000000000000
New Teacher USDC balance: 10500000000000000000000
Contract balance: 7500000000000000000000
The bursary is not updated correctly, but the contract, the principal and teacher USDC balances are updated.
See the following code snippet for the test case:
function test_audit_graduate_bursary_is_not_updated() public schoolInSession {
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
uint256 oldBursary = levelOneProxy.bursary();
uint256 principalUSDCBalance = usdc.balanceOf(principal);
uint256 teacherUSDCBalance = usdc.balanceOf(alice);
console2.log("Old bursary: ", oldBursary);
console2.log("Principal USDC balance: ", principalUSDCBalance);
console2.log("Teacher USDC balance: ", teacherUSDCBalance);
console2.log("Contract balance: ", usdc.balanceOf(address(levelOneProxy)));
vm.startPrank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
vm.stopPrank();
uint256 newBursary = levelOneProxy.bursary();
uint256 newPrincipalUSDCBalance = usdc.balanceOf(principal);
uint256 newTeacherUSDCBalance = usdc.balanceOf(alice);
assertEq(newBursary, oldBursary);
assertGt(newPrincipalUSDCBalance, principalUSDCBalance);
assertGt(newTeacherUSDCBalance, teacherUSDCBalance);
console2.log("New bursary: ", newBursary);
console2.log("New Principal USDC balance: ", newPrincipalUSDCBalance);
console2.log("New Teacher USDC balance: ", newTeacherUSDCBalance);
console2.log("Contract balance: ", usdc.balanceOf(address(levelOneProxy)));
}
Tools Used
Manual review.
Recommendations
Update the bursary calculation in the graduateAndUpgrade function to ensure that the bursary is accurately reflected in the contract balance after the upgrade.
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++) {
+ bursary -= payPerTeacher;
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
+ bursary -= principalPay;
usdc.safeTransfer(principal, principalPay);
}
