Clearwood
commented
3 years ago We are consiouscly using initializers as a pattern. With contracts that are actively used already like the auctions, initializers are called by factories after a clone has been created.
ghoul-sol
Handle
leastwood
Vulnerability details
Impact
There are a few instances whereby a malicious actor could monitor the blockchain for instances of bytecode matching any of MISO's suite of smart contracts. These smart contracts can be logically separated into two types, core and auxiliary contracts. MISO's core contracts are deployed using an automated script and MISO's auxiliary contracts are deployed by users to suit their auction requirements. There is currently no mechanism in place to ensure that a contract is deployed and initialized safely. By using a two-step process, MISO is allowing attackers to frontrun their deployments and potentially DOS further upgrades. While MISO's base contracts have already been deployed, any further upgrades to mitigate vulnerabilities will be prone to frontrunning. Additionally, auxiliary contracts deployed by users are also vulnerable, i.e.
MISOFermenter.solandGnosisSafeFactory. DOS attacks can be sustained by malicious actors as deployment costs far outweigh the cost to frontrun and initialize these contracts.Proof of Concept
https://github.com/sushiswap/miso/blob/master/deploy/main.ts https://github.com/sushiswap/miso/blob/master/contracts/MISOFermenter.sol#L98-L105 https://github.com/sushiswap/miso/blob/master/contracts/Vault/GnosisSafeFactory.sol#L44-L51
Tools Used
Manual code review.
Recommended Mitigation Steps
Contracts implementing access control can be deployed such that their
constructor()assigns the admin role to the deployer account. Upon initialization, this role can be removed from the deployer account and assigned to the correct admin account. This guarantees frontrunning prevention as long as the deployer's private key is not compromised before or during deployment.