Hawk High
First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: medium
Likelihood: low
Invalid

Missing teacher removal restrictions in `LevelOne` contract leads to potential wage evasion

s3bc40

Summary

Depending on the level of confidence on the principal, there is a possibility of wage manipulation. The principal can remove teachers before the LevelOne::graduateAndUpgrade function is called. This can lead to a situation where he can still receive his wage, while the teachers do not receive their wages even if they have participated to the session. Therefore, the principal can manipulate the system and evade their responsibilities.

function removeTeacher(address _teacher) public onlyPrincipal {
if (!isTeacher[_teacher]) {
revert HH__TeacherDoesNotExist();
}
​
// Remove teacher from list
for (uint256 n = 0; n < listOfTeachers.length; n++) {
if (listOfTeachers[n] == _teacher) {
listOfTeachers[n] = listOfTeachers[listOfTeachers.length - 1];
listOfTeachers.pop();
break;
}
}
}

Impact

This issue can bring unfairness to the system and can lead to a situation where the principal can manipulate the system to their advantage. This can lead to a loss of trust in the system and can lead to potential financial losses for the teachers.

Proof of Concept

  1. Principal prepares the next upgrades after the session end.

  2. Principal removes teachers and graduates to LevelTwo before the LevelOne::graduateAndUpgrade function is called.

  3. The principal receives their wage.

  4. The contract balance is updated correctly, but the teachers do not receive their wages.

See the following test case for the PoC:

function test_audit_principal_can_remove_teacher_before_wages_share() public schoolInSession {
// Initialize the LevelTwo implementation
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
// Setup the expected values for the test
uint256 expectedPrincipalWageAmount =
(levelOneProxy.PRINCIPAL_WAGE() * levelOneProxy.bursary()) /
levelOneProxy.PRECISION();
uint256 expectedtContractBalance = levelOneProxy.bursary() - expectedPrincipalWageAmount;
​
// Balances before the upgrade
console2.log("Principal USDC balance before: ", usdc.balanceOf(principal));
console2.log("Teacher Alice USDC balance before: ", usdc.balanceOf(alice));
console2.log("Teacher Bob USDC balance before: ", usdc.balanceOf(bob));
console2.log("Contract balance before: ", usdc.balanceOf(address(levelOneProxy)));
​
// Principal removes teachers and graduates to LevelTwo
vm.startPrank(principal);
levelOneProxy.removeTeacher(alice);
levelOneProxy.removeTeacher(bob);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
vm.stopPrank();
​
// Balances after the upgrade
console2.log("Principal USDC balance before: ", usdc.balanceOf(principal));
console2.log("Teacher Alice USDC balance before: ", usdc.balanceOf(alice));
console2.log("Teacher Bob USDC balance before: ", usdc.balanceOf(bob));
console2.log("Contract balance before: ", usdc.balanceOf(address(levelOneProxy)));
​
// Asert that only the principal has received their wage and the contract balance is updated
assertEq(usdc.balanceOf(alice), 0);
assertEq(usdc.balanceOf(bob), 0);
assertEq(usdc.balanceOf(principal), expectedPrincipalWageAmount);
assertEq(usdc.balanceOf(address(levelOneProxy)), expectedtContractBalance);
}

Tools Used

Manual review.

Recommendations

Ensure that the removeTeacher function is not callable in this specific case. This can be done by adding a check to ensure that the removeTeacher function is not called after the sessionEnd and before the LevelOne::graduateAndUpgrade function is called. This will ensure that the teachers are not removed before the wages are shared and that the system is not manipulated. Any means possible to ensure confidence on the protocol.

Updates

Lead Judging Commences

yeahchibyke Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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

Give us feedback!