Unrestricted initialize() can be frontran

[hats-bug-reporter[bot]]

Github username: @0xDetermination Submission hash (on-chain): 0xdaafb805219e79cfca832079f3497e87547c741602099b0ada8f7ef82bc11d5f Severity: medium

Description:

Impact

An attacker can set the governance address, fee address, and owner address. The contract will need to be redeployed, and the protocol will lose the deployment fees. If the protocol does not notice the attack, then the attacker can steal the contract's funds.

PoC

DappnodeSmoothingPool.sol does not have a constructor, and there is no access control on the initialize() function. Therefore, anyone can call initialize() to set the governance address, fee recipient address, and owner address.

    function initialize( // NOTICE THAT THERE IS NO ACCESS CONTROL
        address _governance,
        uint256 _subscriptionCollateral,
        uint256 _poolFee,
        address _poolFeeRecipient,
        uint64 _checkpointSlotSize,
        uint64 _quorum
    ) external initializer {
        // Initialize requires
        require(
            _poolFee <= 10000,
            "DappnodeSmoothingPool::initialize: Pool fee cannot be greater than 100%"
        );

        require(
            _quorum != 0,
            "DappnodeSmoothingPool::initialize: Quorum cannot be 0"
        );

        // Set initialize parameters
        governance = _governance;

Attack example:

1. Protocol calls initialize()
2. Attacker frontruns the call by calling initialize(), setting governance and owner addresses to the attacker's own address
3. Users send collateral to the protocol
4. Attacker adds an attacker-controlled address as an oracle to the protocol and changes quorum to 1
5. The attacker calls submitReport() and updates the reward root such that the balance of the contract can be transferred to the attacker's address
6. The attacker calls claimRewards() and drains the contract

   Relevant Code

   https://github.com/hats-finance/Smooth-0x64bc275b37e62eec81a00ecaecd2b9567058f990/blob/main/contracts/DappnodeSmoothingPool.sol#L182-L220

   Recommended Fix

   Call initialize() in the constructor, or add access control.

[invocamanman]

Duplicated: cannot be front-runned due the OZ scripts deploys and initialize the proxies in the same transaction https://github.com/hats-finance/Smooth-0x64bc275b37e62eec81a00ecaecd2b9567058f990/issues/21