Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No way for governance to properly update feeProtocolNum and feeProtocolDen #252

Open
c4-bot-4 opened this issue Oct 30, 2024 · 0 comments
Open
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_primary AI based primary recommendation sufficient quality report This report is of sufficient quality

Comments

@c4-bot-4
Copy link
Contributor

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

@c4-bot-4 c4-bot-4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Oct 30, 2024
c4-bot-10 added a commit that referenced this issue Oct 30, 2024
@c4-bot-12 c4-bot-12 added the 🤖_primary AI based primary recommendation label Oct 30, 2024
howlbot-integration bot added a commit that referenced this issue Nov 4, 2024
@howlbot-integration howlbot-integration bot added the sufficient quality report This report is of sufficient quality label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working 🤖_primary AI based primary recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

2 participants