Frontrunning initialization of contract

[code423n4]

Lines of code

https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/contracts/MyStrategy.sol#L56-L74

Vulnerability details

The initialization of MyStrategy.sol can be front-runned and this could cause lose of funds in the worst case or a DoS attack in the best case.

More precisely, the attacker can proceed as following:

1. The malicious actor prepares a fake implementation of Vault.sol with the same interface as IVault.sol, and such that
   1. its .token() function returns address(AURA) to bypass the assert in L57, and
   2. implementing governance(), strategist(), keeper() and guardian() to return his controlled addresses, so he can assume complete control of the deployed MyStrategy.sol.
2. The malicious actor listen the mempool for the deployment of MyStrategy.sol; when he listen the deployment, he calls (or frontrun) the initialize function using the address of his vault.

If the frontrunning is sucessful, two scenarios occurs:

1. In the worst case, the deployer of MyStrategy.sol does not realize that his transaction was front-runned and the malicious actor can withdraw the funds of strategy to his controlled vault after some time.
2. In the best case, this can turns into a DoS attack: the deployer of MyStrategy.sol realizes that his transaction was front-runned and will need to re-deploy the contract again. If the attacker repeats the frontrunning, the deployer will need to repeat the deployment until he wins the attacker in the gas auction.

Impact

High, as this exploit could lead to lose of funds in the worst case.

Proof of Concept

1. The malicious actor prepares a fake implementation of Vault.sol with the same interface as IVault.sol, and such that
   1. its .token() function returns address(AURA) to bypass the assert in L57, and
   2. implementing governance(), strategist(), keeper() and guardian() to return his controlled addresses, so he can assume complete control of the deployed MyStrategy.sol.
2. The malicious actor listen the mempool for the deployment of MyStrategy.sol; when he listen the deployment, he calls (or frontrun) the initialize function using the address of his vault.

Tools Used

Manual analysis

Recommended Mitigation Steps

One possible mitigation is to modify the deployment script to pre-process the contract with the correct address of the vault after the latter has been deployed.

Note: Mitigation using Flashbots Protect RPC is not guaranteed to be sucessful since it does not mitigate against frontrunning by uncle bandit attacks.

[jack-the-pug]

https://github.com/Badger-Finance/vested-aura/blob/d504684e4f9b56660a9e6c6dfb839dcebac3c174/scripts/1_production_deploy.py#L109-L114

Downgraded to QA as the current deploy script will call initialize on the deployment of the proxy contract.