ctf_sec - Two functions in TradingModule.sol both use initializer modifiers, one of the initializer will fail and contract will be not be initialized properly.

[sherlock-admin]

ctf_sec

high

Two functions in TradingModule.sol both use initializer modifiers, one of the initializer will fail and contract will be not be initialized properly.

Summary

Initializer modifiers are used twice in constructor and in initialize function in TradingModule.sol

Vulnerability Detail

The constructor for TradingModule.sol use the initializer modifier

The function initialize inside the TradingModule.sol also use initializer modifier.

But a smart contract can only call initializer once. The second initializer function call will fail

https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable#initializers

However, while Solidity ensures that a constructor is called only once in the lifetime of a contract, a regular function can be called many times. To prevent a contract from being initialized multiple times, you need to add a check to ensure the initialize function is called only once:

Since this pattern is very common when writing upgradeable contracts, OpenZeppelin Contracts provides an Initializable base contract that has an initializer modifier that takes care of this.

Impact

function call

function initialize(uint32 maxOracleFreshnessInSeconds_) initializer

will fails

Code Snippet

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/TradingModule.sol#L41-L45

https://github.com/sherlock-audit/2022-09-notional/blob/main/leveraged-vaults/contracts/trading/TradingModule.sol#L55-L58

Tool used

Manual Review

Recommendation

We recommend the project only use initializer modifier once and complete all the initialization inside the function because contract inherits UUPSUpgradeable proxy.

function initialize