Summary
When graduateAndUpgrade is called and teachers receive their pay for the session, it is not divided by the number of teachers. Instead, the contract sends each teacher the full amount intended for all teachers.
Vulnerability Details
We can see that payPerTeacher is set to (bursary * TEACHER_WAGE) / PRECISION but this does not account for the number of teachers in the current session.
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);
}
This PoC only works if the vulnerablities affecting the storage layout and invalid implementation are fixed, otherwise we can't upgrade the contract and therefore cannot check it's state.
PoC:
function test_teacher_wage_is_incorrect() public {
_teachersAdded();
console2.log(levelOneProxy.TEACHER_WAGE());
console2.log(levelOneProxy.bursary());
vm.startPrank(clara);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
console2.log(levelOneProxy.bursary());
vm.startPrank(principal);
levelOneProxy.startSession(70);
vm.stopPrank();
vm.warp(block.timestamp + 4 weeks);
vm.startPrank(principal);
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
bytes memory data = abi.encodeCall(levelTwoImplementation.graduate, ());
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
levelOneProxy.upgradeToAndCall(levelTwoImplementationAddress, data);
vm.stopPrank();
console2.log(usdc.balanceOf(alice));
console2.log(usdc.balanceOf(bob));
console2.log(levelOneProxy.bursary());
}
Impact
payPerTeacher is not divided by totalTeachers. This means that each teacher gets paid more than they should, effectively draining the contract and breaking the protocol's functionality.
Tools Used
Manual review
Recommendations
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 payPerTeacher = ((bursary * TEACHER_WAGE) / PRECISION) / totalTeachers;
uint256 principalPay = (bursary * PRINCIPAL_WAGE) / PRECISION;
_authorizeUpgrade(_levelTwo);
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
usdc.safeTransfer(principal, principalPay);
}
