pks_ - `Borrower#modify()` function may Dos in some cases

[sherlock-admin2]

pks_

medium

Borrower#modify() function may Dos in some cases

Summary

Borrower#modify() function may Dos in some cases.

Vulnerability Detail

Factory#pause is a public function, this function called Borrower#getPrices() and return seemsLegit parameter. If seemsLegit is false, getParameters[pool].pausedUntilTime can be increased. The pausedUntilTime parameter is used in Borrower#modify() function:

require(
    seemsLegit && (block.timestamp > pausedUntilTime) && (address(this).balance >= ante),
    "Aloe: missing ante / sus price"
);

as we can see, this function will revert if seemsLegit is false or block.timestamp > pausedUntilTime.

The Borrower#getPrices() call trace is shown below:

Borrower#getPrices() -> Borrower#_getPrices() -> ORACLE.consult(UNISWAP_POOL, oracleSeed)

the lastWrites[pool] parameter is updated in VolatilityOracle#update function, one member of lastWrites struct is iv, this parameter is calculated by Volatility.estimate(cachedMetadata[pool], data, a, b, IV_SCALE) function, and the data parameter returned by Oracle#consult function is get from pool.slot0(). However, slot0() is easy to manipulate in uniswap. So malicious user can monitor the mempool and can modify lastWrites[pool] to make seemsLegit = false and increase pausedUntilTime. Or when market volatility is great, the Borrower#modify() is also reverted when user call this function.

Impact

Contract will be reverted in some cases when users call Borrower#modify() function.

Code Snippet

https://github.com/aloelabs/aloe-ii/blob/6fb1d96a1ad5a2913eefa476faf302bf5c4443ed/core/src/Borrower.sol#L478-L483 https://github.com/aloelabs/aloe-ii/blob/6fb1d96a1ad5a2913eefa476faf302bf5c4443ed/core/src/libraries/Oracle.sol#L45 https://github.com/aloelabs/aloe-ii/blob/6fb1d96a1ad5a2913eefa476faf302bf5c4443ed/core/src/VolatilityOracle.sol#L52-L89

Tool used

vscode, Manual Review

Recommendation

To make any calculation use a TWAP instead of slot0.

[sherlock-admin2]

2 comment(s) were left on this issue during the judging contest.

panprog commented:

low, because the seemsLegit is expected to return false when pool seems to be manipulated - this is not easy to achieve and if it does return false/true in some edge case, it's better to be on a safer side and pause trading until it is true. Simple same-block pool manipulation can not make seemsLegit false on its own.

MohammedRizwan commented:

low severity

[Banditx0x]

Escalate. Unrelated to #63 - "IV Can be Decreased for Free."

The suggested impact here is "Contract will be reverted in some cases when users call Borrower#modify() function." which does not relate with the "IV Manipulation" issue this labelled a duplicate of.

The core elements of the valid issue are:

1. depositing a large amount of liquidity into the active tick
2. using this to decrease IV

This issue refers to slot0 price manipulation and seemsLegit, both of which are unrelated to the dupe issue. It should be considered a seperate issue with low severity.

[sherlock-admin2]

Escalate. Unrelated to #63 - "IV Can be Decreased for Free."

The suggested impact here is "Contract will be reverted in some cases when users call Borrower#modify() function." which does not relate with the "IV Manipulation" issue this labelled a duplicate of.

The core elements of the valid issue are:

1. depositing a large amount of liquidity into the active tick
2. using this to decrease IV

This issue refers to slot0 price manipulation and seemsLegit, both of which are unrelated to the dupe issue. It should be considered a seperate issue with low severity.

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.

[yuliyu123]

1. slot0 sqrtPriceX96&currentTick could be manipulated by depositing a large amount of liquidity.
2. The returned data parameter here be used to calculate the iv parameter here inside update function.

The modify function is just one of the affected functions, the key is iv could be manipulated with flashloan.

[cvetanovv]

Check my comment https://github.com/sherlock-audit/2023-10-aloe-judging/issues/10#issuecomment-1813248307

[Banditx0x]

Just addressing the original submission, seemsLegit is the TWAP oracle manipulation detection mechanism, and is designed to pause the modify function when the price has deviated far from the current price in the recent pass. This is not a DOS, but a pause designed by the protocol as a safety mechanism.

[roguereddwarf]

The original submission does only describe the Borrower.modify() function, the information that @yuliyu123 added 4 days ago is not contained in the original submission.

I agree with both @Banditx0x and @panprog (who judged this as invalid in the judging contest) that what is described in the original submission is intended behavior.

The added context from 4 days ago:

The modify function is just one of the affected functions, the key is iv could be manipulated with flashloan.

is correct. But this was not contained in the original submission and simply states again what the parent issue talks about.

[Trumpero]

Planning to accept the escalation, de-duplicate this issue from #63, and mark this issue as invalid.

[Czar102]

Result: Invalid Unique

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• banditx0x: accepted