Uninitialized Implementation Contracts are not Completely Protected

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/main/src/PredyPool.sol#L68
https://github.com/code-423n4/2024-05-predy/blob/main/src/markets/gamma/GammaTradeMarket.sol#L67
https://github.com/code-423n4/2024-05-predy/blob/main/src/markets/perp/PerpMarketV1.sol#L90

Vulnerability details

Impact

The uninitialized implementation contracts are not totally safe against an eventual takeover by an attacker even if their initialize functions use initializer modifier.

Proof of Concept

As a proof of concept, I’ll provide the links to the relevant OpenZeppelin documentation / communication.

According to OpenZeppelin’s documentation:

https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/master/contracts/proxy/utils/Initializable.sol#L43-L45

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.

Here is the link to the respective communication with OpenZeppelin staff:
https://forum.openzeppelin.com/t/what-does-disableinitializers-function-mean/28730/7

Tools Used

Manual checking.

Recommended Mitigation Steps

Invoke _disableInitializers wherever applicable:

// src/PredyPool.sol

contract PredyPool is IPredyPool, IUniswapV3MintCallback, Initializable, ReentrancyGuardUpgradeable {
	...
	constructor() {
+		_disableInitializers();
	}
	...
}

// src/markets/gamma/GammaTradeMarket.sol

contract GammaTradeMarket is IFillerMarket, BaseMarketUpgradable, ReentrancyGuardUpgradeable {
	...
	constructor() {
+		_disableInitializers();
	}
	...
}

// src/markets/perp/PerpMarketV1.sol

contract PerpMarketV1 is BaseMarketUpgradable, ReentrancyGuardUpgradeable {
	...
	constructor() {
+		_disableInitializers();
	}
	...
}

Assessed type

Upgradable