Submission Details
Severity:
[H] `LevelOne.sol#graduateAndUpgrade()` does not invoke upgradeable function
Vulnerability Details
Principal can call LevelOne.sol#graduateAndUpgrade to upgrade the system at the end of the school session. However, the function does not invoke UUPSUpgradeable.sol#upgradeToAndCall(), which results in the system still using the original implementation.
Impact
The functionality of upgrade is broken.
PoC
// SPDX-License-Identifier: SEE LICENSE IN LICENSE
import {Test, console2, Vm} from "forge-std/Test.sol";
import {DeployLevelOne} from "../script/DeployLevelOne.s.sol";
import {GraduateToLevelTwo} from "../script/GraduateToLevelTwo.s.sol";
import {LevelOne} from "../src/LevelOne.sol";
import {LevelTwo} from "../src/LevelTwo.sol";
import {MockUSDC} from "./mocks/MockUSDC.sol";
contract LevelOneAndGraduateTest is Test {
DeployLevelOne deployBot;
GraduateToLevelTwo graduateBot;
LevelOne levelOneProxy;
LevelTwo levelTwoImplementation;
address proxyAddress;
address levelOneImplementationAddress;
address levelTwoImplementationAddress;
MockUSDC usdc;
address principal;
uint256 schoolFees;
// teachers
address alice;
address bob;
// students
address clara;
address dan;
address eli;
address fin;
address grey;
address harriet;
function setUp() public {
deployBot = new DeployLevelOne();
proxyAddress = deployBot.deployLevelOne();
levelOneProxy = LevelOne(proxyAddress);
// graduateBot = new GraduateToLevelTwo();
usdc = deployBot.getUSDC();
principal = deployBot.principal();
schoolFees = deployBot.getSchoolFees();
levelOneImplementationAddress = deployBot.getImplementationAddress();
alice = makeAddr("first_teacher");
bob = makeAddr("second_teacher");
clara = makeAddr("first_student");
dan = makeAddr("second_student");
eli = makeAddr("third_student");
fin = makeAddr("fourth_student");
grey = makeAddr("fifth_student");
harriet = makeAddr("six_student");
usdc.mint(clara, schoolFees);
usdc.mint(dan, schoolFees);
usdc.mint(eli, schoolFees);
usdc.mint(fin, schoolFees);
usdc.mint(grey, schoolFees);
usdc.mint(harriet, schoolFees);
}
function test_poc_upgradeable() external {
// levelOneImplementationAddress
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
// stateDiff for levelOneProxy.getSchoolFeesToken()
vm.startStateDiffRecording();
levelOneProxy.getSchoolFeesToken();
Vm.AccountAccess[] memory levelOneRecord = vm.stopAndReturnStateDiff();
// call graduateAndUpgrade()
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
// stateDiff for levelTwoProxy.getSchoolFeesToken()
vm.startStateDiffRecording();
levelTwoProxy.getSchoolFeesToken();
Vm.AccountAccess[] memory levelTwoRecord = vm.stopAndReturnStateDiff();
// record[0].account is proxy address, should equal
assertTrue(levelOneRecord[0].account == levelTwoRecord[0].account);
// record[1].account is proxy address, should not equal
assertFalse(levelOneRecord[1].account == levelTwoRecord[1].account);
}
}
Log shows that assertion for record[1].account is the same. After calling graduateAndUpgrade, proxy still makes a delegatecall to LevelOne.sol.
[FAIL: assertion failed] test_poc_upgradeable() (gas: 579428)
Traces:
[579428] LevelOneAndGraduateTest::test_poc_upgradeable()
├─ [445078] → new LevelTwo@0x2e234DAe75C793f67A35089C9d99245E1C58470b
│ └─ ← [Return] 2223 bytes of code
├─ [0] VM::startStateDiffRecording()
│ └─ ← [Return]
├─ [3579] ERC1967Proxy::fallback() [staticcall]
│ ├─ [605] LevelOne::getSchoolFeesToken() [delegatecall]
│ │ └─ ← [Return] MockUSDC: [0x104fBc016F4bb334D775a19E8A6510109AC63E00]
│ └─ ← [Return] MockUSDC: [0x104fBc016F4bb334D775a19E8A6510109AC63E00]
├─ [0] VM::stopAndReturnStateDiff()
│ └─ ← [Return] [((0, 31337 [3.133e4]), 3, 0x90193C961A926261B756D1E5bb255e67ff9498A1, 0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496, true, 0, 0, 0x, 0, 0x930fa4ec, false, [(0x90193C961A926261B756D1E5bb255e67ff9498A1, 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc, false, 0x00000000000000000000000034a1d3fff3958843c43ad80f30b94c510645c316, 0x00000000000000000000000034a1d3fff3958843c43ad80f30b94c510645c316, false)], 1), ((0, 31337 [3.133e4]), 1, 0x34A1D3fff3958843C43aD80F30b94c510645C316, 0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496, true, 0, 0, 0x, 0, 0x930fa4ec, false, [(0x90193C961A926261B756D1E5bb255e67ff9498A1, 0x000000000000000000000000000000000000000000000000000000000000000c, false, 0x000000000000000000000000104fbc016f4bb334d775a19e8a6510109ac63e00, 0x000000000000000000000000104fbc016f4bb334d775a19e8a6510109ac63e00, false)], 2)]
├─ [0] VM::prank(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca])
│ └─ ← [Return]
├─ [10719] ERC1967Proxy::fallback(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca)
│ ├─ [10230] LevelOne::graduateAndUpgrade(LevelTwo: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0xd3618cca) [delegatecall]
│ │ ├─ [3850] MockUSDC::transfer(principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca], 0)
│ │ │ ├─ emit Transfer(from: ERC1967Proxy: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: principal: [0x6b9470599cb23a06988C6332ABE964d6608A50ca], value: 0)
│ │ │ └─ ← [Return] true
│ │ └─ ← [Stop]
│ └─ ← [Return]
├─ [0] VM::startStateDiffRecording()
│ └─ ← [Return]
├─ [1079] ERC1967Proxy::fallback() [staticcall]
│ ├─ [605] LevelOne::getSchoolFeesToken() [delegatecall]
│ │ └─ ← [Return] MockUSDC: [0x104fBc016F4bb334D775a19E8A6510109AC63E00]
│ └─ ← [Return] MockUSDC: [0x104fBc016F4bb334D775a19E8A6510109AC63E00]
├─ [0] VM::stopAndReturnStateDiff()
│ └─ ← [Return] [((0, 31337 [3.133e4]), 3, 0x90193C961A926261B756D1E5bb255e67ff9498A1, 0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496, true, 0, 0, 0x, 0, 0x930fa4ec, false, [(0x90193C961A926261B756D1E5bb255e67ff9498A1, 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc, false, 0x00000000000000000000000034a1d3fff3958843c43ad80f30b94c510645c316, 0x00000000000000000000000034a1d3fff3958843c43ad80f30b94c510645c316, false)], 1), ((0, 31337 [3.133e4]), 1, 0x34A1D3fff3958843C43aD80F30b94c510645C316, 0x7FA9385bE102ac3EAc297483Dd6233D62b3e1496, true, 0, 0, 0x, 0, 0x930fa4ec, false, [(0x90193C961A926261B756D1E5bb255e67ff9498A1, 0x000000000000000000000000000000000000000000000000000000000000000c, false, 0x000000000000000000000000104fbc016f4bb334d775a19e8a6510109ac63e00, 0x000000000000000000000000104fbc016f4bb334d775a19e8a6510109ac63e00, false)], 2)]
├─ [0] VM::assertTrue(true) [staticcall]
│ └─ ← [Return]
├─ [0] VM::assertFalse(true) [staticcall]
│ └─ ← [Revert] assertion failed
└─ ← [Revert] assertion failed
Tools Used
Manual.
Recommendations
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);
> upgradeToAndCall(_levelTwo, data);
for (uint256 n = 0; n < totalTeachers; n++) {
usdc.safeTransfer(listOfTeachers[n], payPerTeacher);
}
usdc.safeTransfer(principal, principalPay);
}
Updates
Lead Judging Commences
yeahchibyke 6 months ago
Submission Judgement Published Assigned finding tags:
failed upgrade
The system doesn't implement UUPS properly.
yeahchibyke 6 months ago
Submission Judgement Published Assigned finding tags:
failed upgrade
The system doesn't implement UUPS properly.
Rewards breakdown
nSLOC
243This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.