0xKartikgiri00 - Not Calling _disableInitializers() in `ProxyFactory` contract constructor can lead to exploit.

[sherlock-admin2]

0xKartikgiri00

medium

Not Calling _disableInitializers() in ProxyFactory contract constructor can lead to exploit.

Summary

The ProxyFactory contract is not calling _disableInitializers() function in its constructor, can lead to the reinitialization attack by the attacker. Cause the initialize function can be called in implementation contract which in this case is ProxyFactory.

Vulnerability Detail

It is important to call that the _disableInitializers() function in the constructor of the implementation contract so that initialize function is only called once during deployment to initialize the contract's state by proxy. OpenZeppelin's Initializable contract provides the _disableInitializers() function, which prevents any subsequent calls to the initializer functions of the implementation contract after it has been initialized by the proxy. However, this function is not being called in the constructor of the ProxyFactory contract, which leads to the reinitialization attacks by reintializing the admin, implementaion contract and proxy.

So we don't need that the initialize function is called again once the proxy is deployed. As it is in the OpenZeppelin's Initializable contract. "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"

Impact

Not calling _disableInitializers() in the constructor of the ProxyFactory contract allows attacker to reintialized the admin, implementaion contract and proxy.

Code Snippet

https://github.com/sherlock-audit/2024-02-telcoin-platform-audit-update/blob/main/telcoin-contracts/contracts/factories/ProxyFactory.sol#L35

Tool used

Manual Review

Recommendation

The recommended mitigation is to call _disableInitializers() function in the constructor of the ProxyFactory contract . This will prevent the initialization function from being called more than once.

Recommendation in code:-

 /// @custom:oz-upgrades-unsafe-allow constructor
  constructor() {
      _disableInitializers();
  }

[sherlock-admin3]

2 comment(s) were left on this issue during the judging contest.

WangAudit commented:

as 45; as Watson said themselves; _disableInitializers should be called in implementation contract; not the proxy; also seems very vague; not about our exact codebase and initializer modifier solves the problems

takarez commented:

invalid