Summary

In the contracts that implement Openzeppelin’s Upgreadable model, uninitialized Implementation contract can be taken over by an attacker with initialize() function.

Example LLMOracleManager contract.

Vulnerability Details

Scenario:

1. Proxy & Implementation are deployed.

2. The Proxy delegates calls to Implementation.initialize() which sets the owner and switches initialized to true in the state of the Proxy.

3. The storage of Implementation however is still intact e.g owner is unset and initialized is false.

4. An attacker calls initialize() directly on Implementation and sets himself as the owner.

5. From here, he has full control to perform any maliceous activities.

Impact

Take-over by an attacker means contract sabbotage allowing him to execute privileged activities maliceously.

Tools Used

Manual Review

Recommendations

From Openzeppelin Docs:

Do not leave an implementation contract uninitialized. An uninitialized implementation contract can be taken over by an attacker, which may impact the proxy. To prevent the implementation contract from being used, you should invoke the _disableInitializers function in the constructor to automatically lock it when it is deployed: