hats-finance / Smooth-0x64bc275b37e62eec81a00ecaecd2b9567058f990

Dappnode's MEV Smoothing Pool
0 stars 2 forks source link

Contract's initialization can be frontran #21

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: -- Submission hash (on-chain): 0xdab76ab679e709b0be65cb2c86b71025045e3e8882016c2d3df1f3aae4d3395e Severity: low

Description: Description

During deployment phase, an attacker can frontrun the initialize function.

Attack Scenario

An attacker front runs the contract and sets critical contract parameters (governance, poolfee reciepient etc). If the frontrun is noticed, a redeployment is needed, causing financial costs. If the frontrun isn't noticed, users could start using a system which is compromised from the start

Attachments

  1. Proof of Concept (PoC) File

https://github.com/dappnode/mev-sp-contracts/blob/3929e24ea288d697d38948b8690c8c2028e5042b/contracts/DappnodeSmoothingPool.sol#L182

function initialize(
        address _governance,
        uint256 _subscriptionCollateral,
        uint256 _poolFee,
        address _poolFeeRecipient,
        uint64 _checkpointSlotSize,
        uint64 _quorum
    ) external initializer {

Anyone can call this function and set the parameters.

  1. Revised Code File (Optional)

An admin option can be added, e.g onlyOwner.

function initialize(
        address _governance,
        uint256 _subscriptionCollateral,
        uint256 _poolFee,
        address _poolFeeRecipient,
        uint64 _checkpointSlotSize,
        uint64 _quorum
    ) external initializer onlyOwner {
invocamanman commented 1 year ago

The librearies of OpenZeppelin deploys a proxy and in that smae transaction initializes them. Just for giving extra information, this is the constructor of the proxy, which is called with the initialization function and parameters https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/proxy/ERC1967/ERC1967Proxy.sol#L27