VaultProxy initialise can be frontrun

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/main/contracts/VaultProxy.sol#L17 https://github.com/code-423n4/2023-06-stader/blob/main/contracts/VaultProxy.sol#L20

Vulnerability details

Impact

VaultProxy::initialise can be called by anyone and it can be frontrun. In OpenZeppelin, proxy contracts prevent this by using disableInitializers in the constructor, as you know of. VaultProxy::initialise sets important parameters for the Vault and can only be called once, so if this is frontrun by a malicious actor, developer should deploy another VaultProxy.

Furthermore, since VaultProxy deployment is done in VaultFactory::initialize, this will ultimately lead to redeployment of VaultFactory. https://github.com/code-423n4/2023-06-stader/blob/main/contracts/factory/VaultFactory.sol#L29

    function initialize(address _admin, address _staderConfig) external initializer {
        ...
        vaultProxyImplementation = address(new VaultProxy());

        _grantRole(DEFAULT_ADMIN_ROLE, _admin);
    }

Proof of Concept

1. Deployer calls the deployContracts.ts script to deploy contracts. VaultFactory gets deployed.
2. Attacker gets the VaultFactory address and listens for initialized event.
3. Deployer later calls VaultFactory::initialize and creates VaultProxy.
4. Attacker gets the event and call VaultProxy::initialise before the deployer. This is possible since deployer should call deployWithdrawVault or deployNodeELRewardVault in most likely another tx, considering those functions have onlyRole(NODE_REGISTRY_CONTRACT) modifier.

Assessed type

DoS

[code423n4]

Withdrawn by dwward3n