Udsen - MALICIOUS USER CAN FRONT RUN THE `initialize()` FUNCTION IN THE `OpenQV1` AND BECOME THE OWNER OF THE CONTRACT

[github-actions[bot]]

Udsen

high

MALICIOUS USER CAN FRONT RUN THE initialize() FUNCTION IN THE OpenQV1 AND BECOME THE OWNER OF THE CONTRACT

Summary

OpenQV1 is an UUPS Upgradeable contract. It's an implementation contract and expected to call via OpenQProxy.sol. But the OpenQProxy.sol in its constructor only defines the implementation contract address and dosen't pass in the _data field to initialize the OpenQV1 initialization() function. (as per the following comment an empty byte array in the case of OpenQ given in the OpenQProxy.sol contract) .

Vulnerability Detail

Since the initialization() function of the OpenQV1 is not called during contract deployment of OpenQProxy.sol, an external user will have to initialize the OpenQV1 via a delegateCall through the proxy contract. Hence any malicious user can front run this initialization delegateCall via the OpenQProxy and become the owner of the contract thus depriving the OpenQ protocol the opportunity to be the owner.

Impact

A malicious user can be the owner of the OpenQV1 implementation contract and following onlyOwner callable functions inside the OpenQV1 contract will be vulnerable as a result.

setBountyFactory setClaimManager setDepositManager _authorizeUpgrade transferOracle

Code Snippet

    /// @param _data Additional data to pass to initialize method (an empty byte array in the case of OpenQ)
    constructor(address _logic, bytes memory _data)
        payable
        ERC1967Proxy(_logic, _data) //@audit-issue - _data should initialize the implementation contract. Else anyone could initialize the implementation contract and become the owner.
    {}

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/OpenQ/Proxy/OpenQProxy.sol#L12-L16

    function initialize() external initializer onlyProxy { 
        __Ownable_init();
        __UUPSUpgradeable_init();
        __ReentrancyGuard_init();
    } 

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/OpenQ/Implementations/OpenQV1.sol#L14-L18

Tool used

VS Code and Manual Review

Recommendation

Provide the data field in the constructor (not an empty byte array as suggested in the comment) of the OpenQProxy contract during deployment and provide the keccack256 hash of function signature of the initialize() function as _data to initialize the OpenQV1 implementation contract at once. Thus depriving an attacker to front run the initialization of the OpenQV1 contract.