hats-finance / VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f

LP token lending protocol
MIT License
2 stars 1 forks source link

Upgradable contracts should have an initialization function #9

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: -- Beneficiary: 0x32cb680634a33d107FDe4CE3b0EF21F07158701B Submission hash (on-chain): 0x547cd06bc6b1009088bb35eb62a4e97d614031a3248ec8e2f3e46461eeb89324 Severity: medium severity

Description:

Summary

Upgradable contracts should have an initialization function

Vulnerability Detail

Upgradable contracts in Solidity should have an initialization function to ensure proper setup, maintain state consistency, and enhance security. Unlike regular contracts, upgradable contracts use a proxy pattern that separates the contract's logic from its data storage, enabling seamless updates to the logic without affecting the stored data. However, this approach requires careful initialization of the contract state to avoid inconsistencies or vulnerabilities.

Code Snippet

LendingPoolCollateralManager.sol: 35-35

33: contract LendingPoolCollateralManager is
34:     ILendingPoolCollateralManager,
35:     VersionedInitializable, // <= FOUND
36:     LendingPoolStorage
37: {

Recommendation

Resolution: Implement an initialization function in your upgradable contracts to establish a secure and accurate initial state. This function should include setting all the initial variable values and any necessary setup operations. To further enhance safety, consider using a modifier such as initializer from OpenZeppelin's Initializable contract, which ensures the initialization function can only be called once. Additionally, always conduct thorough testing and reviews on your initialization process to ensure it's correctly preparing your contract state, mitigating any potential inconsistencies or vulnerabilities that could arise from an incorrect initial setup.

ksyao2002 commented 1 year ago

LendingPoolCollateralManager.sol is only used as a logic function for the LendingPool.sol to delegatecall. Because of this, it doesn't require initialization as there is no state needed to be initialized.

Check Aave's codebase for reference: https://github.com/aave/protocol-v2/blob/master/contracts/protocol/lendingpool/LendingPoolCollateralManager.sol

/**