Zaros Part 1

Summary

The uninitialized implementation contract is not totally safe against an eventual takeover by an attacker even if their initialize() functions use initializer modifier.

Vulnerability Details

UpgradeBranch contract is meant to be updreadable:

However, the contract lacks means to automatically lock it when it is deployed so as to prevent takeover by an attacker.

According to OpenZeppelin’s documentation:

An uninitialized contract can be taken over by an attacker. This applies to both a proxy and its implementation contract, 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.

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

Without the lock, an attacker can take over the contract and perform any malicious activities.

Tools Used

Manual Review

Recommendations

Invoke _disableInitializers() wherever applicable: