Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Curve and Uniswap Adapters can not be initialized because the `__BaseAdapter_init(...)` uses `initializer` modifier instead of onlyInitializing

kaysoft

Summary

The BaseAdapter.sol contract's initialization function uses initializer modifier instead of onlyInitializing modifier.

This will cause invocation of the initialize(...) of the of the child contracts to revert because they also use the initialize modifier and the initialize modifier is supposed to be used only once.

Vulnerability Details

Only the final child contract in the inheritance chain is supposed to use the initializemodifier while the parent contracts use the onlyInitializingmodifier because the initializemodifier is supposed to be called only once.

The BaseAdapter.sol is the parent contract to CurveAdapter.sol, UniswapV3Adapter.sol and UniswapV2Adapter.sol

The BaseAdapter.sol uses the initializermodifier on its __BaseAdapter_init(...)initialization function while the child contracts that inherit the base contract BaseAdapter.sol also use the initializermodifier. This will cause the intialization of the CurveAdapter.sol, UniswapV3Adapter.sol and UniswapV2Adapter.sol contracts to revert.

Not initializing this mentioned contract will prevent setting the ownerwhich will in turn make the onlyOwner functions useless.

Also the _slippageToleranceBpscan not be set causing all swap transaction to revert because the default value is zero.

File: BaseAdapter.sol
function __BaseAdapter_init(address owner, uint256 _slippageToleranceBps) public initializer {
// initialize the owner
__Ownable_init(owner);
​
// set the slippage tolerance
setSlippageTolerance(_slippageToleranceBps);
}
File: CurveAdapter.sol
function initialize(
address owner,
address _curveStrategyRouter,
uint256 _slippageToleranceBps
)
external
initializer
{
// initialize the owner
__BaseAdapter_init(owner, _slippageToleranceBps);
​
// set the Curve Swap Strategy Router
setCurveStrategyRouter(_curveStrategyRouter);
}
File: UniswapV2Adapter.sol
function initialize(
address owner,
address _uniswapV2SwapStrategyRouter,
uint256 _slippageToleranceBps
)
external
initializer
{
// initialize the owner
__BaseAdapter_init(owner, _slippageToleranceBps);
​
// set the Uniswap V2 Swap Strategy Router
setUniswapV2SwapStrategyRouter(_uniswapV2SwapStrategyRouter);
}
File: UniswapV3Adapter.sol
function initialize(
address owner,
address _uniswapV3SwapStrategyRouter,
uint256 _slippageToleranceBps,
uint24 _fee
)
external
initializer
{
// initialize the owner
__BaseAdapter_init(owner, _slippageToleranceBps);
​
// set the Uniswap V3 Swap Strategy Router
setUniswapV3SwapStrategyRouter(_uniswapV3SwapStrategyRouter);
​
// set the pool fee
setPoolFee(_fee);
}

Here is the comment on the initialize modifier indicating that the initializemodifier can only be called once and the parent contract should use the onlyInitializingmodifier.

/**
* @dev A modifier that defines a protected initializer function that can be invoked at most once. In its scope,
* `onlyInitializing` functions can be used to initialize parent contracts.
*
* Similar to `reinitializer(1)`, except that in the context of a constructor an `initializer` may be invoked any
* number of times. This behavior in the constructor can be useful during testing and is not expected to be used in
* production.
*
* Emits an {Initialized} event.
*/
modifier initializer() {
...
}

Impact

  1. CurveAdapter.sol, UniswapV2Adapter.sol and UniswapV3Adapter.sol cannot be initialized.

  2. The above contracts cannot be upgraded.

  3. deadline for swap cannot be set in the dex adapters

Tools Used

Manual Review

Recommendations

Consider changing the initializer modifier to onlyInitializing modifier on __BaseAdapter_init(...) of BaseAdapter.sol this way:

-- function __BaseAdapter_init(address owner, uint256 _slippageToleranceBps) public initializer {
++ function __BaseAdapter_init(address owner, uint256 _slippageToleranceBps) public onlyInitializing {
// initialize the owner
__Ownable_init(owner);
​
// set the slippage tolerance
setSlippageTolerance(_slippageToleranceBps);
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

BaseAdapter contract uses initializer instead of onlyInitializing modifier, causing initialization to fail in child DEX adapters and breaking swaps

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!