Missing `_disableInitializers` in constructor of `CvxAssetStakerBuffer.sol`.

[hats-bug-reporter[bot]]

Github username: @erictee2802 Twitter username: 0xEricTee Submission hash (on-chain): 0x35c800c8e798e958679ca69555f93c4028b5abb3049192a17c10a3494a6a9199 Severity: low

Description: Missing _disableInitializers in constructor of CvxAssetStakerBuffer.sol.

Description

OpenZeppelin's comment in their documentation:

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.

Attack Scenario

The initialize function in CvxAssetStakerBuffer.sol is unprotected and an attacker is able to initialize the contract. The uninitialized contract can be taken over by the attacker for example by front-running the original deployer's intialize() call. This applies to both the proxy and its implementation contract.

Attachments

NA

1. Proof of Concept (PoC) File

In CvxAssetStakerBuffer.sol, noticed there is no _disableInitializers in constructor.

2. Revised Code File (Optional)

Consider to add _disableInitializers() function and the constructor to CvxAssetStakerBuffer.sol:

++ constructor() {
++  _disableInitializers();
++ }

[PlamenTSV]

A taken over implementation after the dencun is of no risk, since the implementation cannot be self destructed. It would lack any validation to interact with other core contracts, since it needs to be approved by the control tower, so I see no real risk. I see how it is simply an inconvenience, and I can lean towards low, but I will leave it to the sponsor

[0xEricTee]

@PlamenTSV Noticed that other upgradeable contracts implement _disableInitializers function to disallow the initialization of the implementation contract by random addresses, I don't see any reason why this contract don't add this as well for consistency.

[PlamenTSV]

@PlamenTSV Noticed that other upgradeable contracts implement _disableInitializers function to disallow the initialization of the implementation contract by random addresses, I don't see any reason why this contract don't add this as well for consistency.

Yes, it is generally a good practice that was mandatory before the dandun update, but since the removal of selfdestruct exploiting this is impossible without direct delegatecalls. Falls more into info imo. The sponsor may think otherwise.