search

code-423n4 / 2021-09-sushitrident-2-findings

0 stars 0 forks source link

Sanity check on the lower and upper ticks #93

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

broccoli

Vulnerability details

Impact

In the burn and swap functions of ConcentratedLiquidityPool, the lower tick is not explicitly checked to be less than the upper tick. Besides, the ticks are not checked to be at least the minimum tick and at most the maximum tick.

Proof of Concept

Referenced code: Ticks.sol#L68-L70

Recommended Mitigation Steps

Add sanity checks on the lower and upper ticks in critical functions (see the referenced line of code, for example).

sarangparikh22 commented 2 years ago

This doesn't seem reachable, can you give a POC?

alcueca commented 2 years ago

@sarangparikh22, I don't understand what it is that you mean by not reachable, could you please elaborate?

sarangparikh22 commented 2 years ago

If we provide an invalid tick condition, such as lower is higher than upper, it would fail at the FullMath check, as it would overflow and we would get a transaction revert. However, it is better to fail early with a proper message. What I meant by unreachable is, I am unable to find a condition where this condition holds true and we are able to swap or burn with wrong tick specs. We'll make the changes to fail early by checking ticks, and will mark this issue as valid, since it makes sense to just check it early, even though it would fail. @alcueca