Denial of service in deployNewPool

[c4-bot-1]

Lines of code

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

Vulnerability details

Impact

Denial of service in deployNewPool function.

Proof of Concept

For frontrunning protection purposes, there is a check that the "salt" provided equals the caller of the deployNewPool function:

   // frontrunning protection for mined pool addresses
        if (address(bytes20(salt)) != msg.sender) revert Errors.InvalidSalt();

This means that the address of the caller of the deployNewpool function must be equal to the "salt" passed in as parameter.

However, right below the above if statement is another check that restricts the caller of the deployNewPool function to only the factory contract owner:

  // restrict pool deployment to owner if contract has not been made permissionless
   address _owner = s_owner;
        if (_owner != address(0) && _owner != msg.sender) revert Errors.NotOwner();

This is done with the intention that if the contract has not been made permissionless, only the factory contract owner can call deployNewPool function.

Where the problem lies is that there is no check for the condition that the contract has been made permissionless before running the below if statement:

   if (_owner != address(0) && _owner != msg.sender) revert Errors.NotOwner();

This will make the function revert when called by a user.

For instance, when the contract is permissionless and anyone can call deployNewPool, the first if statement above will pass with "salt" set to msg.sender. But the second if statement will revert because the caller is not the factory owner.

Tools Used

Manual review

Recommended Mitigation Steps

The below if statement should be inside a block that first check if the contract is not permissionless first before running the if statement:

  // restrict pool deployment to owner if contract has not been made permissionless
   address _owner = s_owner;
        if (_owner != address(0) && _owner != msg.sender) revert Errors.NotOwner();

Assessed type

DoS

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid

[Picodes]

When the contract is permissionless then the only condition is on the salt ?