No way for governance to properly update feeProtocolNum and feeProtocolDen

[howlbot-integration]

Lines of code

https://github.com/ronin-chain/katana-v3-contracts/blob/03c80179e04f40d96f06c451ea494bb18f2a58fc/src/core/KatanaV3Factory.sol#L105
https://github.com/ronin-chain/katana-v3-contracts/blob/03c80179e04f40d96f06c451ea494bb18f2a58fc/src/core/KatanaV3Factory.sol#L123
https://github.com/ronin-chain/katana-v3-contracts/blob/03c80179e04f40d96f06c451ea494bb18f2a58fc/src/core/KatanaV3Pool.sol#L253
https://github.com/ronin-chain/katana-v3-contracts/blob/03c80179e04f40d96f06c451ea494bb18f2a58fc/src/core/KatanaV3Pool.sol#L261-L262

Vulnerability details

Proof of Concept

feeProtocolNum and feeProtocolDen cannot be updated in slot0 struct if needed by pool governance due to an equivalent function not being available, and if done through factory, will not register in the struct leading to operations being conducted with stale parameters.

There's no equivalent function to update the feeProtocolAmounts causing that feeProtocolNum and feeProtocolDen cannot be directly updated in the pool (unlike can be done in uniswap).

Although, KatanaV3Factory has the enableFeeAmount which allows the owner to set, or potentially update the feeProtocolNum and feeProtocolDen parameters,

  function _enableFeeAmount(uint24 fee, int24 tickSpacing, uint8 feeProtocolNum, uint8 feeProtocolDen) private {
    feeAmountTickSpacing[fee] = tickSpacing;
>>  feeAmountProtocol[fee] = Fraction(feeProtocolNum, feeProtocolDen);
    emit FeeAmountEnabled(fee, tickSpacing, feeProtocolNum, feeProtocolDen);
  }

the update, while registered in the factory will not be updated in the slot0 struct and as a result, pool operations will still be conducted based on the previous stale feeProtocolNum and feeProtocolDen.

Recommended Mitigation Steps

Add a relevant governance function to update the feeAmountProtocol in slot0 based on the enabled feeAmount. Or introduce a stand-alone function similar to uniswap's setFeeProtocol

Assessed type

Context

[khangvv]

For clarity, we removed the setFeeProtocol function from Uniswap, as we moved the fee tier enablement feature to the Factory, determined before pool deployments. In the updated code, enableFeeAmount is intended only to set new fee tiers for future pools and does not affect any deployed pools (see reference).

If you have specific concerns about removing setFeeProtocol and the potential risks, please leave an update comment.

[alex-ppg]

The submission states that it is not possible to retroactively change the fee imposed on a particular pool once it has been established. This behavior is in line with the business requirements of the current system and I do not envision any vulnerability arising from it.

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid