sherlock-audit / 2024-05-sophon-judging

7 stars 6 forks source link

Ragnark_323 - Dangerous Proxy Pattern Implementation with Constructor and Immutable Variables Leading to Incompatibility #198

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

Ragnark_323

medium

Dangerous Proxy Pattern Implementation with Constructor and Immutable Variables Leading to Incompatibility

Summary

The SophonFarming.sol contract uses a constructor with immutable variables, which is incompatible with the proxy pattern. Move state initialization to an initialize function to ensure proper setup via the proxy.

Vulnerability Detail

The SophonFarming.sol contract currently uses a constructor to initialize immutable variables. This approach is incompatible with the proxy pattern used for upgradeable contracts. The constructor is executed only once when the implementation contract is deployed, and its state changes are not reflected in the proxy's storage. Additionally, immutable variables are stored in the bytecode, meaning their values are shared across all proxies pointing to the same implementation contract, which is not desirable.

Impact

Immutable Variables: Immutable variables set in the constructor are stored in the bytecode and shared across all proxies. This can lead to unintended behavior as each proxy should have its own state.

Code Snippet

https://github.com/sherlock-audit/2024-05-sophon/blob/05059e53755f24ae9e3a3bb2996de15df0289a6c/farming-contracts/contracts/farm/SophonFarming.sol#L64C5-L88C1

Tool used

Manual Review

Recommendation

To ensure compatibility with the proxy pattern, remove the constructor and use an initializer function to set up the contract state. This function should be called after the proxy is deployed and points to the implementation contract.

Links https://docs.openzeppelin.com/upgrades-plugins/1.x/proxies#the-constructor-caveat:~:text=the%20developer%20appropriately.-,The%20Constructor%20Caveat,-In%20Solidity%2C%20code

https://docs.openzeppelin.com/upgrades-plugins/1.x/faq#why-cant-i-use-immutable-variables:~:text=pattern%20here.-,Why%20can%E2%80%99t%20I%20use%20immutable%20variables%3F,-Solidity%200.6.5%20introduced

sherlock-admin4 commented 5 months ago

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

0xmystery commented:

invalid because the boolean _initialized is used instead