Part 2

Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Submission Details
Severity: low
Valid

BaseAdapter::__BaseAdapter_init Should Use onlyInitializing, Not initializer

Summary

The BaseAdapter is a parent contract being inherited by 3 other adapter contracts. It's initializer function __BaseAdapter_init is executed inside the scope of the other 3 adapter contracts. Accoding to openzeppelin doc12, __BaseAdapter_init should use onlyInitializing modifier as it is executed inside the scope of an initializer.


Impact

Violates the recommended approach of openzeppelin and it cannot be initialized after proxy contract deployment


PoC

Paste the following code in test/contest/ProxyTest.t.sol and run forge test --mt testInit:

// SPDX-License-Identifier: SEE LICENSE IN LICENSE
pragma solidity ^0.8.20;
import {Test, console} from "forge-std/Test.sol";
import {CurveAdapter} from "@zaros/utils/dex-adapters/CurveAdapter.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {Initializable} from "@openzeppelin/contracts/proxy/utils/Initializable.sol";
contract ProxyTest is Test {
CurveAdapter curveAdapter;
ERC1967Proxy proxy;
CurveAdapter curveAdapterProxy;
address mockOnwer = makeAddr("mockowner");
uint256 mockSlippageToleranceBps = 5000;
address mockCurveStrategyRouter = makeAddr("mockCurveStrategyRouter");
function setUp() public {
curveAdapter = new CurveAdapter();
}
function testInit() public {
proxy = new ERC1967Proxy(address(curveAdapter), ""); // Proxy deployment
curveAdapterProxy = CurveAdapter(address(proxy));
vm.prank(mockOnwer);
vm.expectRevert(abi.encodeWithSelector(Initializable.InvalidInitialization.selector));
curveAdapterProxy.initialize(mockOnwer, mockCurveStrategyRouter, mockSlippageToleranceBps); // Proxy initialization
}
}

Recommendation

Change initializer to onlyInitializing in __BaseAdapter_init:

- 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!