cergyk - HOT::setPriceBounds Malicious executor can brick vault withdrawals for at least 2 days

[sherlock-admin4]

cergyk

medium

HOT::setPriceBounds Malicious executor can brick vault withdrawals for at least 2 days

Summary

The HOT::setPriceBounds function in the HOT contract allows an executor to set the AMM position's square-root upper and lower price bounds. A malicious executor can exploit this function to set the bounds very close together, causing an uint128 overflow when calculating liquidity. This results in disabling vault withdrawals, requiring a timelock of 2 days to set a new executor and re-enable vault withdrawals (by setting other price bounds).

Vulnerability Detail

Please note that this exploit currently works for all pools with at least one rebasing token.

The HOT::setPriceBounds function can be used by the executor to set the AMM position's square-root upper and lower price bounds: HOT.sol#L520-L574.

A malicious executor can set _sqrtPriceLowX96 and _sqrtPriceHighX96 very close together. The following constraint is checked: _sqrtPriceLowX96 < sqrtPriceSpotX96 < _sqrtPriceHighX96, so they must be spaced by at least 2: HOTParams.sol#L177-L180.

If the difference is so small, while computing liquidity in _calculateAMMLiquidity(), the cast toUint128 in LiquidityAmounts.sol#L31 would revert even for small amounts.

While setting price bounds the executor has to compute values close enough to the limit, but not overflowing, so the _calculateAMMLiquidity() at the end of setPriceBounds() succeeds: HOT.sol#L571.

As the call to setPriceBounds has succeeded, the executor can donate a small amount of rebasing token directly to the pool. As the reserve of rebasing token is the balance of this token, this makes all subsequent calls to _calculateAMMLiquidity revert.

As a result, all functions of HOT.sol become unusable, but most notably the withdrawals since this means that user funds are locked during that time.

The executor has then to donate a small amount of the rebasing token to the pool. This would result in the returned uint128 liquidity to overflow (even for reasonable values of amount0 and sqrtRatioAX96): LiquidityAmounts.sol#L31, LiquidityAmounts.sol#L48, thus bricking vault withdrawals.

Example:

Given:

• sqrtRatioAX96 = 1 << 96
• sqrtRatioBX96 = 1 << 96 + 2

It would overflow for a amount equal to 2 33 (which is smaller than 1 ether == 1018).

To unbrick the vault withdrawals, an owner would have to set a new executor who would set new price bounds, which means a timelock of 2 days has to pass.

Impact

Vault withdrawals would be bricked for at least 2 days.

Code Snippet

• HOT.sol#L1015-L1024

Tool used

Manual Review

Recommendation

Implement additional constraints on the setPriceBounds function to ensure that the _sqrtPriceLowX96 and _sqrtPriceHighX96 values cannot be set too close together, preventing potential overflow in the liquidity calculation.

UniswapV3 naturally does not have this issue, since price ranges can only lie on ticks

[IWildSniperI]

quoting this from sherlock judging rules

this issue doesn't identify a valid attack path since its not describing how an executor would actually call HOT::setPriceBounds()

[IWildSniperI]

quoting this from readme, liquidity provider is trusted

[sherlock-admin3]

escalate During the audit, I also noticed this issue. However, for it to be exploited, it requires a DoS attack lasting 7 days or more and at least 3 malicious executors(sherlock rule: https://docs.sherlock.xyz/audits/judging/judging#:~:text=Could%20Denial%2Dof,Medium%20severity%20issue.). Although the conditions for this exploitation are quite extreme, and repeated occurrences may lead to users losing trust in the Vault, it does need to be fixed.

You've deleted an escalation for this issue.

[cu5t0mPeo]

escalate During the audit, I also noticed this issue. However, for it to be exploited, it requires a DoS attack lasting 7 days or more and at least 3 malicious executors(https://docs.sherlock.xyz/audits/judging/judging#:~:text=Could%20Denial%2Dof,Medium%20severity%20issue.). Repeated occurrences of this issue could lead to users losing trust in the Vault. Therefore, I believe the exploitation conditions are quite extreme, and it should be classified as a low-level issue, but it still needs to be fixed.

[sherlock-admin3]

escalate During the audit, I also noticed this issue. However, for it to be exploited, it requires a DoS attack lasting 7 days or more and at least 3 malicious executors(https://docs.sherlock.xyz/audits/judging/judging#:~:text=Could%20Denial%2Dof,Medium%20severity%20issue.). Repeated occurrences of this issue could lead to users losing trust in the Vault. Therefore, I believe the exploitation conditions are quite extreme, and it should be classified as a low-level issue, but it still needs to be fixed.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[kfx]

Why would the withdrawals get blocked? The call to _calculateAMMLiquidity in HOT::withdrawLiquidity is done after removing the tokens from the pool. Withdrawing a sufficient proportion of the liquidity will un-DoS the vault. https://github.com/sherlock-audit/2024-03-arrakis/blob/main/valantis-hot/src/HOT.sol#L736

[WangSecurity]

Firstly, indeed the funds can be unlock in less than 7 days. Secondly, as I understand it's liquidity provider who calls the setPriceBounds function, who is trusted based on the README. Excuse me if I'm wrong, planning to accept the escalation and invalidate the report.

[CergyK]

@WangSecurity

Firstly, indeed the funds can be unlock in less than 7 days. Secondly, as I understand it's liquidity provider who calls the setPriceBounds function, who is trusted based on the README. Excuse me if I'm wrong, planning to accept the escalation and invalidate the report.

Firstly, the seven days rule is applied for DoS of functionality:

Here the actual impact is not DoSing the functionality, but temporary freezing of funds for at least 2 days (no LP can recover their funds during that time). One can check that the withdraw function in HOT.sol is otherwise coded in a way to never make a withdrawal revert if it is legitimate.

Second the onlyLiquidityProvider check only ensures that this function is called through the Valantis module, and can actually be called by the executor (RESTRICTED): ValantisHOTModule.sol#L294-L314

ArrakisStandardManager.sol#L376

[WangSecurity]

Thank you for clarification about liquidity providers.

But for the first point, the rule about DoS still applies:

Could Denial-of-Service (DOS), griefing, or locking of contracts count as a Medium (or High) issue? DoS has two separate scores on which it can become an issue: i. The issue causes locking of funds for users for more than a week.

Hence, the decision remains the same, planning to accept the escalation and invalidate the report.

[WangSecurity]

Result: Invalid Unique

[sherlock-admin4]

Escalations have been resolved successfully!

Escalation status:

• cu5t0mPeo: accepted