[Bug]: Bad Owner Can Brick initialize By Setting A Bad protocolFeeController

[Philogy]

Describe the bug

A malicious/compromised owner of the PoolManager contract could set a protocolFeeController via the setProtocolFeeController method such that PoolManager.initialize always reverts, making it impossible to create new pools. This could compromise integrating protocols which may expect the initialize method to always be successfully callable (assuming that other pre-conditions are met).

A bad protocol fee controller can cause initialize to revert in 2 ways:

1. Return a fee parameter that's in an invalid range
2. Cause the try { ... } catch { ... } block to revert by returning data that doesn't decode into a (uint8, uint8) pair as expected.

Expected Behavior

Expected behavior would be that the PoolManager gracefully falls back to a default fee of 0 as the try { ... } catch { ... } block in _fetchProtocolFees implies. Alternatively, it should be made more explicit that the owner can globally shut down the creation of new pools via the initialize function.

To Reproduce

1. Copy this PoC (https://gist.github.com/Philogy/6a7e4360a677cb38b477542d94222bc5) into test/foundry-tests/BrickInitialize.t.sol
2. Run forge test --mp test/foundry-tests/BrickInitialize.t.sol -vvvv
3. See that the testCanBrick() test passes with initialize reverting in 2 separate cases solely due to a bad protocol fee controller.

Note: The PoC showcases both methods of causing initialize to revert.

Additional context

No response

[Philogy]

Note the owner could also use this to selectively censor the creation of pools by sandwiching calls to initialize with protocol fee controller changes

[marktoda]

Nice catch, thanks for looking and reporting!