MorpheusAI

MorpheusAI
Foundry
22,500 USDC
View results
Submission Details
Severity: medium
Valid

Due to no access control on `DistributionV2::_authorizeUpgrade()` anyone can change the implementation contract and can destroy the main Proxy contract.

Vulnerability Details :

DistributionV2 contract inheriting UUPSUpgradeable so overriding _authorizeUpgrade() function. But adding no access control to this function can lead to setting implementation contract by anyone since this function is called for authorization in UUPSUpgradeable. By setting any malicious contract as implementation attacker can selfdestruct() the proxy contract. Since implementation is called using delegatecall from proxy.

Code Snippet :

contracts/mock/DistributionV2.sol

36: function _authorizeUpgrade(address) internal view override {}

Since DistributionV2 contract is inheriting UUPSUpgradeable from openzeppelin we can see in upgrading implementation address _authorizeUpgrade() is called.

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/proxy/utils/UUPSUpgradeable.sol#L86C5-L90C1

function upgradeToAndCall(address newImplementation, bytes memory data) public payable virtual onlyProxy {
_authorizeUpgrade(newImplementation);
_upgradeToAndCallUUPS(newImplementation, data);
}

Malicious DistributionV2 implementation contract

contract DistributionV2 is UUPSUpgradeable {
IDistribution.Pool[] public pools;
function version() external pure returns (uint256) {
return 2;
}
function createPool(IDistribution.Pool calldata pool_) public {
selfdestruct(address(0));//@audit this code will be implanted here
}
function getPeriodReward(uint256 poolId_, uint128 startTime_, uint128 endTime_) public view returns (uint256) {
IDistribution.Pool storage pool = pools[poolId_];
return LinearDistributionIntervalDecrease.getPeriodReward(
pool.initialReward, pool.rewardDecrease, pool.payoutStart, pool.decreaseInterval, startTime_, endTime_
);
}
function _authorizeUpgrade(address) internal view override {}
}

First attacker will call upgradeToAndCall to main proxy contract which will change the implementation contract address to his malicious DistributionV2 contract.
Now Attacker will call createPool to main proxy contract of DistributionV2 and by delegatecall proxy will call implementation contract .It will destroy the main proxy contract using selfdestruct(). And erase all the storage from main proxy contract of DistributionV2.

Impact :

The main proxy contract will be destroyed and erase all the storage from main proxy contract of DistributionV2.

Tools Used

Manual Review

Recommended Mitigation Steps :

Add proper access control to _authorizeUpgrade() function since it is used for authorization at the time of changing implementation contract address. This can be mitigated by using 'OwnableUpgradeable' of openzeppelin and add onlyOwner modifier to the _authorizeUpgrade() function so. Only owner can be authorized to upgrade the implementation address. Add one initalizer function also to initialize the owner.

File : contracts/mock/DistributionV2.sol
...
+ import {OwnableUpgradeable} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
- contract DistributionV2 is UUPSUpgradeable {
+ contract DistributionV2 is OwnableUpgradeable, UUPSUpgradeable {
...
+ function Distribution_init() external initializer
+ {
+ __Ownable_init();
+ __UUPSUpgradeable_init();
+ }
...
- function _authorizeUpgrade(address) internal view override {}
+ function _authorizeUpgrade(address) internal view override onlyOwner {}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

_authorizeUpgrade(address) lacks access control in mock contracts

0x11singh99 Submitter
over 1 year ago
inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

_authorizeUpgrade(address) lacks access control in mock contracts

Support

FAQs

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