sherlock-audit / 2023-02-openq-judging

4 stars 0 forks source link

Udsen - `initialize()` FUNCTION IS NOT PROTECTED BY `onlyProxy` MODIFIER #512

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

Udsen

high

initialize() FUNCTION IS NOT PROTECTED BY onlyProxy MODIFIER

Summary

initialize() function in the DepositManagerV1 contract is not protected by the onlyProxy modifier. Since initialize() function is declared as an external function any external user can front run and call this function for the first time and become the owner of the contract. This will make both onlyOwner functions in the contract setTokenWhitelist() and _authorizeUpgrade() functions vulnerable.

Vulnerability Detail

The DepositManagerV1 contract is defined as a UUPS Upgradeable contract and expected to be called via the OpenQProxy.sol. But the initialize() function is not protected by the onlyProxy modifier. Thus making it possible for any external user to call this function and become the owner of the contract by front-running any other transaction for initialization.

Impact

The missing onlyProxy modifier in the initialize() function can lead an attacker to get the ownership of the contract. Thus making the two onlyOwner functions setTokenWhitelist() and _authorizeUpgrade() vulnerable for attacks.

Code Snippet

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

https://github.com/sherlock-audit/2023-02-openq/blob/main/contracts/DepositManager/Implementations/DepositManagerV1.sol#L15-L18

Tool used

VS Code and Manual Review

Recommendation

Apply the onlyProxy modifier to the initialize() function in the DepositManagerV1 contract as below:

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