code-423n4 / 2024-01-salty-findings

11 stars 6 forks source link

Attacker Can Inflate LP Position Value To Create a Bad Debt Loan #222

Open c4-bot-3 opened 8 months ago

c4-bot-3 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L95

Vulnerability details

Impact

An attacker can inflate the value of liquidity through reserve ratio manipulation (even without manipulating the aggregated oracle) and take out a undercollateralized USDS loan.

Proof of Concept

The value of a liquidity position in Salty is determined by:

  1. Checking the reserves of the pool/liquidity position (easy to manipulate)
  2. Valuing the tokens that comprise the liquidity position using an aggregated oracle (difficult to manipulate)

The only invariant enforced by the Uniswap v2 style AMM is $x * y = k$. When the ratio of the tokens deviates from the "correct" ratio (where the ratio corresponds exactly to the correct value of the tokens), the liquidity will always be worth more , as calculated by the USD value of the individual tokens. There is the well known frontrunning attack on liquidity deposits which works based on this fact.

Mathematical Example

Let's go through a simple example. The pool is USDT-DAI, which is selected for simplicity as they are both tokens with 18 decimals and the oracle always returns a value of $1 for both tokens.

Currently there is 1e18 tokens in each pool, and let's say that the constant k is 1e36

Invariant formula: $x * y = k$

$$ x = 1e18\

y = 1e18\

k = 1e36\ $$

Let's do a swap which doubles the x variable and recalculate the corresponding y value:

$$ 2e18 * y = 1e36 \

y = 0.5e18 \

(x = 2e18) $$

Now if we apply the correct oracle price of the tokens, which is $1 per 1e18 wei, the value of the LP position is now worth (0.5 + 2) = $2.5 , which is more , than the correct value of $2.

An intuitive explanation of value inflation

We know that large trades that push the reserve ratio away from the correct ratio loses money for traders (this money is gained by LP's). Trades that move the reserves towards the correct ratio gain money for traders (denominated by the "real value" of the tokens). This is the basis for AMM arbitrage and slippage attacks.

Pushing the price far from the correct reserve ratio can thus be seen as a temporary inflation of the price of the LP token when the token reserves are valued by their USD denominated price. This inflation comes from the manipulating trader losing a large amount of USD-denominated value.

Using this intuitive analogy, we can see why it's only possible to inflate the LP price and not deflate it. Pushing the reserve ratio towards the correct ratio is the method of deflating the USD-denominated value, however, the non-manipulated value should always approximately match the correct price, so it is not possible!

Escalating to an Attack

The LP token is used as collateral to borrow USDS, so we can inflate the LP price to borrow more than the liquidity position's real worth. The attacker can then dump the USDS and the whole USDS stablecoin will be undercollateralized and collapse.

Does Automated Arbitrage Prevent Pool Manipulation?

There is potential difficulty in manipulating the Salty pool due to the automated arbitrage. If there is enough liquidity in the pools in the arbitragePath, then the protocol will swap WETH to more WETH through the path which will rebalance the pool.

Note that this requires "enough liquidity" in the pool path. One could simply deposit a large amount liquidity in the pool being manipulated such that it holds far more token value than the other pools. Therefore, the WETH arbitrage will leave the pools imbalanced due to causing alot of price slippage in the smaller pools to create only a small price correction in the large pool.

Note that the potential profit of the attacker, which is to borrow all the USDS available at a massively discounted rate, is huge. Therefore the loss of losing to the WETH arbitrage would be negligible compared to how much they gain.

Tools Used

Manual Review

Recommended Mitigation Steps

Valuing LP positions is tricky. Salty's current method is to "trust" the untrustworthy pool reserves. Instead, you could value the liquidity as if the ratio was at the correct ratio, rather than the current pool ratio. Here is an example of a protocol implementing this solution for Uniswap v3 positions:

https://github.com/arcadia-finance/accounts-v2/blob/main/src/asset-modules/UniswapV3/UniswapV3AM.sol

Assessed type

Uniswap

c4-judge commented 8 months ago

Picodes marked the issue as duplicate of #945

c4-judge commented 8 months ago

Picodes marked the issue as selected for report

c4-judge commented 8 months ago

Picodes marked the issue as satisfactory

Picodes commented 8 months ago

The sponsor's answer to this issue can be found here: https://github.com/code-423n4/2024-01-salty-findings/issues/945#issuecomment-1939769047

As shown by this report, this attack seems valid but if arbitrage paths are properly configured the cost of attack is greater than usual. You'd need to either increase the size of the pool or manipulate the price of all the pools in the arbitrage paths at once to prevent arbitrages from happening. As it's still possible, the correct severity seems to be Medium under " leak value with a hypothetical attack path with stated assumptions, but external requirements.", the external requirements being that the cost of attack is smaller than the value extractable by borrowing USDS.

c4-sponsor commented 8 months ago

othernet-global marked the issue as disagree with severity

othernet-global commented 8 months ago

USDS has been removed from the exchange.

https://github.com/othernet-global/salty-io/commit/f3ff64a21449feb60a60c0d60721cfe2c24151c1

c4-judge commented 8 months ago

Picodes changed the severity to 2 (Med Risk)

c4-sponsor commented 8 months ago

othernet-global (sponsor) confirmed

othernet-global commented 8 months ago

The stablecoin framework: /stablecoin, /price_feed, WBTC/WETH collateral, PriceAggregator, price feeds and USDS have been removed: https://github.com/othernet-global/salty-io/commit/88b7fd1f3f5e037a155424a85275efd79f3e9bf9