Lines of code

Vulnerability details

Impact

HIGH - Assets can be lost directly
Anybody can initialize the Vault’s implementation contract. The worst case would be to selfdestruct and make all the (already deployed and to be deployed) Vault’s proxies useless and assets in the deployed proxies will be lost.

Proof of Concept

• proof of concept
  • Notes: part of destroying code is in the setUp, due to limitation of foundry. The code size will be checked in the testInitAndDestroy_poc.

In the proof of concept, alice calls init on the Vault implementation and takes the ownership (line 38). Alice can now install a malicious plugin, the SelfdestructPlugin located in the above of the test contract (line 6-10). Then simply calling the destroy function on the Vault implementation (line 54). Alice could simply renounce her ownership to zero address to bypass the check on ownership change.

The main cause is the Vault implementation was not initialized. The deploy scripts also do not call init on deployed Vault implementation. It is safer and easier to init right after the contract is deployed.

// VaultFactory::constructor
// Vault implementation is deployed but not initialized

 20     constructor() {
 21         implementation = address(new Vault());
 22     }

The problem could be exploited even further because any functionality could be installed as plugins.
Also lack of zero address check for transferring ownership made it simple to bypass the check on ownership change.

Tools Used

foundry

Recommended Mitigation Steps

Add init after deploy the vault implementation.

// VaultFactory::constructor
// to mitigate, add init after deploy

 20     constructor() {
 21         implementation = address(new Vault());
 //	    implementation.init();
 22     }  

---

The text was updated successfully, but these errors were encountered:

👀 1 ecmendenhall reacted with eyes emoji

All reactions

• 👀 1 reaction