Submission Details
Severity:
The ` graduateAndUpgrade ` function does not actually upgrade
Summary
When a principal calls the graduateAndUpgrade function, the contract does not actually upgrade.
Vulnerability Details
The vulnerability is located in the graduateAndUpgrade() function of the LevelOne.sol file.
function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
...
_authorizeUpgrade(_levelTwo);
...
}
function _authorizeUpgrade(address newImplementation) internal override onlyPrincipal {}
The _authorizeUpgrade() function merely authorizes the upgrade, but does not actually upgrade the implementation contract.
Proof Of Concept
Test Case
function testNotRealUpgrade() public schoolInSession {
bytes32 implementationSlot = bytes32(0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc);
// before upgrade
bytes32 slotValue = vm.load(address(levelOneProxy), implementationSlot);
address implementationAddress = address(uint160(uint256(slotValue)));
assertEq(implementationAddress, address(levelOneImplementationAddress));
// upgrade
levelTwoImplementation = new LevelTwo();
levelTwoImplementationAddress = address(levelTwoImplementation);
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
vm.prank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
LevelTwo levelTwoProxy = LevelTwo(proxyAddress);
// after upgrade
slotValue = vm.load(address(levelTwoProxy), implementationSlot);
implementationAddress = address(uint160(uint256(slotValue)));
assertNotEq(implementationAddress, address(levelTwoImplementationAddress));
// the same address as before
assertEq(implementationAddress, address(levelOneImplementationAddress));
}
Output
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.75ms (2.02ms CPU time)
Ran 1 test suite in 1.34s (7.75ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Impact
Contract does not actually upgrade.
Recommendations
Directly use the upgradeToAndCall() function from OpenZeppelin.
// function graduateAndUpgrade(address _levelTwo, bytes memory) public onlyPrincipal {
function graduateAndUpgrade(address _levelTwo, bytes memory data) public onlyPrincipal {
...
// _authorizeUpgrade(_levelTwo);
upgradeToAndCall(_levelTwo, data);
...
}
function _authorizeUpgrade(address newImplementation) internal override onlyPrincipal {}
Of course, the LevelTwo.sol file also needs to be modified slightly.
// SPDX-License-Identifier: MIT
// add
import {UUPSUpgradeable} from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
// contract LevelTwo is Initializable {
contract LevelTwo is Initializable, UUPSUpgradeable {
// add
modifier onlyPrincipal() {
if (msg.sender != principal) {
revert HH__NotPrincipal();
}
_;
}
// add
function _authorizeUpgrade(address newImplementation) internal override onlyPrincipal {}
}
Updates
Lead Judging Commences
yeahchibyke 3 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.