code-423n4 / 2021-11-overlay-findings

1 stars 0 forks source link

Uniswap pool can be manipulated to exploit OverlayV1OVLCollateral's unwind and liquidate functions #142

Closed code423n4 closed 2 years ago

code423n4 commented 3 years ago

Handle

hyh

Vulnerability details

Impact

Using Uniswap feed as sole method of price discovery can be dangerous as pool manipulation possibility is well known and its costs are straightforwardly calculated, so whenever the benefit of unwind and liquidate exploit overweights this cost the vulnerability becomes economically viable and exploitable.

An attacker can setup a bot to observe the conditions, executing the attack whenever profitable.

Proof of Concept

Both unwind and liquidate functions do not check the prices obtained and take actions immediately upon request. Also, both functions do not control for EOA use, allowing flash loans to be used in such manipulations.

Unwind:

https://github.com/code-423n4/2021-11-overlay/blob/main/contracts/collateral/OverlayV1OVLCollateral.sol#L273

Exploit POC: Track the pool and Overlay market, calculating costs vs expected profit, then:

  1. Open a position
  2. Manipulate the pool for position to show profit
  3. Unwind for OVL
  4. Close manipulation transaction: close pool manipulation position, return borrowed funds
  5. Repeat (1-4) while profitable at expected OVL market liquidation price
  6. Dump all the OVL gathered

Liquidate:

https://github.com/code-423n4/2021-11-overlay/blob/main/contracts/collateral/OverlayV1OVLCollateral.sol#L358

Exploit POC: Track the pool and Overlay market positions, calculating costs vs expected profit for each, then:

  1. Manipulate the pool for chosen position to be open for liquidation
  2. Liquidate
  3. Close manipulation transaction: close pool manipulation position, return borrowed funds

Recommended Mitigation Steps

Economic viability of such an attacks can be tricky to track when project start to grow, so it is better to insulate against such manipulations with TWAP Oracle checks and request-action timing separation.

TWAP Oracle check means that whenever price is obtained it is checked vs average and certain operations are prohibited while the divergence is too big.

Request-action separation can be implemented as a query, where users request actions, which take place after some time, at the end of the current period. This way an attacker needs to manipulate the market for extended amount of time, which is much harder overall and prohibits flash loan usage by construction.

mesozoic-technology commented 3 years ago

The prices we consume are indeed the TWAP prices, so I am not sure what this issue is talking about.

Furthermore much of the protocol is in place to guard against abuse of the TWAP oracles. The market impact and dynamic OI caps are explicitly formulated to constrain illiquidity vulnerability, and the bid/ask is designed to guard against natural lagging dynamics of TWAPS.

These protections are frontloaded on entry because we want to avoid a poor user experience altering the payout.

mikeyrf commented 2 years ago

sponsor disputed reason - see oracle attack bullet point in areas of concern. Specifically, assume the spot pool has substantially spread liquidity or minimum level of PCV for longer tailed assets. Means general manipulation of TWAP above bid/ask spread, mentioned in this issue, isn't of concern for assumptions stated

dmvt commented 2 years ago

I agree with the sponsor. This is out of scope and already implemented in the way the warden recommends mitigating the stated issue.