ArkProject: NFT Bridge
ArkProject
NFTBridge
60,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

`UUPSOwnableProxied` is missing `initialize()` function

0xtheblackpanther

Github

https://github.com/Cyfrin/2024-07-ark-project/blob/273b7b94986d3914d5ee737c99a59ec8728b1517/apps/blockchain/ethereum/src/UUPSProxied.sol#L14

Summary

The UUPSOwnableProxied contract inherits from OpenZeppelin's Ownable and UUPSUpgradeable contracts, intending to provide a convenient ownable UUPS proxy. However, the contract lacks an initializer function to set the initial owner, leaving the contract without a defined owner upon deployment.

Impact

Without an initial owner set:

  1. Functions protected by the onlyOwner modifier cannot be executed by any user, rendering these functions unusable.

  2. The contract cannot be properly managed or upgraded as intended, leading to potential security & operational risks.

Proof of Concept

  • Deploy the UUPSOwnableProxied contract.

  • Attempt to call any onlyOwner function ( e.g., _authorizeUpgrade ).

  • The function will revert since no owner is set.

Recommendation

Implement an initializer function to set the initial owner during the first deployment. This ensures that the owner is correctly set and the onlyOwner functions can be used as intended.

New code should look like this:

// SPDX-License-Identifier: MIT
​
pragma solidity ^0.8.0;
​
import "openzeppelin-contracts/contracts/access/Ownable.sol";
import "openzeppelin-contracts/contracts/proxy/utils/UUPSUpgradeable.sol";
import "openzeppelin-contracts-upgradeable/proxy/utils/Initializable.sol";
​
error NotSupportedError();
error NotPayableError();
​
/**
@title Convenient contract to have ownable UUPS proxied contract.
*/
contract UUPSOwnableProxied is Initializable, Ownable, UUPSUpgradeable {
​
// Mapping for implementations initialization.
mapping(address => bool) private _initializedImpls;
​
// onlyInit
modifier onlyInit() {
address impl = _getImplementation();
require(!_initializedImpls[impl], "Already init");
_initializedImpls[impl] = true;
​
_;
}
​
/**
@notice Initializes the contract setting the deployer as the initial owner.
@param initialOwner The initial owner of the contract.
*/
function initialize(address initialOwner) public initializer {
_transferOwnership(initialOwner);
}
​
/**
@notice Only owner should be able to upgrade.
*/
function _authorizeUpgrade(address newImplementation) internal override onlyOwner { }
​
/**
@notice Ensures unsupported function is directly reverted.
*/
fallback() external payable {
revert NotSupportedError();
}
​
/**
@notice Ensures no ether is received without a function call.
*/
receive() external payable {
revert NotPayableError();
}
}

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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