Core Contracts

Summary

The StabilityPool contract implements Openzeppelin’s upgradeable pattern (OwnableUpgradeable, PausableUpgradeable) but uses the ReentrancyGuard instead of ReentrancyGuardUpgradeable which is supposed to be used in upgradeable contracts. It also does not call _disableInitializers in the constructor which could be exploited by an attacker.

Vulnerability Details

• The StabilityPooluses ReentrancyGuard instead of ReentrancyGuardUpgradeable:

• The contract doesn't lock the initializer functions after contract deployment, leaving the contract vulnerable to an attacker that could front run the initialize function:

  // Constructor

  constructor(address initialOwner) {

  _initialOwner = initialOwner;

  }

Impact

Using ReentrancyGuard in an upgradeable contract can lead to several critical issues:

Initialization Failure:

• No constructor means the reentrancy state won't be properly set up

• Potential vulnerability to reentrancy attacks

Storage Collision:

• Non-upgradeable version uses constructor-based storage initialization

• Upgradeable contracts require specific storage layout initialization

Compilation/Deployment Errors:

• Likely to cause compilation errors due to incompatible initialization mechanisms

• Upgradeable contracts require explicit initializer methods

Correct approach is always to use ReentrancyGuardUpgradeable in proxy/upgradeable contracts, ensuring proper initialization and storage management.

Leaving the contract uninitialized could be used by the attacker to potentially hijack the contracts core functionalities.

Tools Used

• Manual Review

Recommendations

1. Replace ReentrancyGuard with ReentrancyGuardUpgradeable

2. Lock the contract, to prevent any future reinitialization:

   /// @custom:oz-upgrades-unsafe-allow constructor

   constructor() {

   _disableInitializers();

   }

3. Adjust the initialize function: