code-423n4 / 2024-10-ronin-findings

0 stars 0 forks source link

`initialize()` function lacks a check to ensure that the provided `sqrtPriceX96` is non-zero before assigning it to `slot0.sqrtPriceX96` #74

Closed howlbot-integration[bot] closed 3 weeks ago

howlbot-integration[bot] commented 4 weeks ago

Lines of code

https://github.com/ronin-chain/katana-v3-contracts/blob/03c80179e04f40d96f06c451ea494bb18f2a58fc/src/core/KatanaV3Pool.sol#L255-L264

Vulnerability details

initialize() function in KatanaV3Pool.sol lacks a crucial check to ensure that the provided sqrtPriceX96 parameter is non-zero. This oversight allows the pool to be initialized with an invalid price state, violating a fundamental invariant of the system.

The reason is that in the initialize function, we directly assign the sqrtPriceX96 parameter to slot0.sqrtPriceX96 without any validation: KatanaV3Pool.sol#L255-L264

slot0 = Slot0({
  sqrtPriceX96: sqrtPriceX96,  // <= @audit This line allows setting sqrtPriceX96 to zero
  // ...
});

This assignment happens without any prior check to ensure sqrtPriceX96 is non-zero.

Impact

Some operations might consistently revert due to division by zero, effectively rendering the pool unusable.

Proof of Concept

Minimal PoC to demonstrate the issue

// Deploy KatanaV3Pool
KatanaV3Pool pool = new KatanaV3Pool();

// Initialize with zero price
pool.initialize(0);

// Check the initialized price
(uint160 sqrtPriceX96,,,,,,) = pool.slot0();
assert(sqrtPriceX96 == 0); // This assertion will pass, indicating the bug

Recommended Mitigation Steps

Implement a check in the initialize() function to ensure sqrtPriceX96 is non-zero

function initialize(uint160 sqrtPriceX96) external override {
  require(slot0.sqrtPriceX96 == 0, "AI");
+ require(sqrtPriceX96 > 0, "Invalid sqrtPriceX96");

  int24 tick = TickMath.getTickAtSqrtRatio(sqrtPriceX96);

  // ...
}

Assessed type

DoS

c4-judge commented 3 weeks ago

alex-ppg marked the issue as not a duplicate

alex-ppg commented 3 weeks ago

The Warden attempts to identify an issue with how the square root price is initialized, however, the TickMath::getTickAtSqrtRatio function will properly ensure a valid ratio has been provided.

c4-judge commented 3 weeks ago

alex-ppg marked the issue as unsatisfactory: Invalid

c4-judge commented 3 weeks ago

alex-ppg marked the issue as primary issue

c4-judge commented 3 weeks ago

alex-ppg marked the issue as unsatisfactory: Invalid