c4-bot-2 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/price_feed/PriceAggregator.sol#L108-L146

Vulnerability details

Bug Description

In the Salty protocol, a combination of bugs across various components can be exploited by a malicious actor to liquidate healthy positions in the market. This complex exploit involves manipulating price feed and exploiting low liquidity in Salty's AMM pools.

1. Stablecoin

The protocol's USDS stablecoin, pegged to WBTC and WETH, allows users to deposit collateral and mint USDS. If the collateral value falls below 110%, the positions are subject to liquidation.

2. Pricefeeds

Salty protocol's valuation of collateral relies on three price feeds: Salty's own USDS/WBTC pool, Uniswap's USDC/WBTC pool, and Chainlink's USD/BTC price feed. The system requires at least two of these feeds to provide non-zero values for a valid price assessment. However, a critical flaw arises when either Chainlink's feed returns an incorrectly low value or the Uniswap TWAP returns an incorrectly low value. This could happen due to a flash crash. The protocol's price aggregator, in this case would be picking the 2 closest prices to calculate the average price.

function _aggregatePrices( uint256 price1, uint256 price2, uint256 price3 ) internal view returns (uint256)
    {
    uint256 numNonZero;

    if (price1 > 0)
        numNonZero++;

    if (price2 > 0)
        numNonZero++;

    if (price3 > 0)
        numNonZero++;

    // If less than two price sources then return zero to indicate failure
    if ( numNonZero < 2 )
        return 0;

    uint256 diff12 = _absoluteDifference(price1, price2);
    uint256 diff13 = _absoluteDifference(price1, price3);
    uint256 diff23 = _absoluteDifference(price2, price3);

    uint256 priceA;
    uint256 priceB;

    if ( ( diff12 <= diff13 ) && ( diff12 <= diff23 ) )
        (priceA, priceB) = (price1, price2);
    else if ( ( diff13 <= diff12 ) && ( diff13 <= diff23 ) )
        (priceA, priceB) = (price1, price3);
    else if ( ( diff23 <= diff12 ) && ( diff23 <= diff13 ) )
        (priceA, priceB) = (price2, price3);

    uint256 averagePrice = ( priceA + priceB ) / 2;

    // If price sources are too far apart then return zero to indicate failure
    if (  (_absoluteDifference(priceA, priceB) * 100000) / averagePrice > maximumPriceFeedPercentDifferenceTimes1000 )
        return 0;

    return averagePrice;
    }

So that means that if an attacker is able to use the moment of the flash crash in chainlink or uniswap and reduce the salty price very low, he will put the aggregator into a state where it returns a average price which is very close to zero (ignoring the 3rd normal price). Using this low price the attacker can now liquidate before healthy position.

3. Manipulating Salty

As salty is a very new AMM, liquidity in pools will be very low at the start. This would allow an attacker to get a flashloan and use it to destabilize the USDS/WBTC pool (which we will consider our attack vector for this case) until it has a very low price. If this is done by just swapping huge amounts, it will have a rather high cost as the arbitrage will refill the pool until the USDS/WETH pool is also highly skewed. This is indeed possible with a flashloan but will require higher funds.

// swap: swapTokenIn->swapTokenOut
// arb: WETH->swapTokenOut->swapTokenIn->WETH
return (swapTokenOut, swapTokenIn);

Luckily there is an easy way to circumvent the automated arbitrage which can be used on low liquidity pools. When one looks at the arbitrage functionality in the Pools.sol file one can see that the atomic arbitrage only occurs if the value of the token that was swapped in is < 100 WETH (wei).

if ( swapAmountInValueInETH <= PoolUtils.DUST )
    return 0;
}

Abusing this functionality, a user can do thousands or millions of small swaps until the target price is reached. As salty has no fees the user can swap back all the funds the the same way only incurring minor losses due to rounding.

4. Liquidating

Now that the price is wildly skewed the attacker can start liquidating before healthy positions and receive 5% of each positions collateral. A small bonus is, that as he has devalued the collateral, the requirement of him getting at max maxRewardValueForCallingLiquidation has also been circumvented and he can receive more rewards (depending on how far he has devalued the collateral).

5. Back to normal

The malicious user can now use the rewards he reaped to cancel out the minor losses he had in the manipulation due to rounding and return the USDS/WBTC pool. Then he can repay the full flashloan. He can also keep the rest of his stolen rewards for himself.

Impact

The issue allows a malicious attacker to liquidate healthy positions in the case of either chainlink or uniswap experiencing a flash crash or returning incorrect values. As the whole system is based on the 2/3 rule to protect against these cases one would expect that the system is protected, which is unfortunately not the case as the Salty pricefeed can easily be manipulated.

Proof of Concept

This POC shows an exemplary scenario. It simulates a case in which Chainlink has incorrectly reported the value of USD/ETC for one block and an attacker monitoring the price feed has detected this. From this point on an attacker can start with the following steps (explained in more detail above):

Acquire a flashloan of WETH
Swap it for USDS (possibly in smaller tranches of 100 to reduce cost) in the salty pool
Liquidate now unhealthy positions and reap the rewards
Stabilize the USDS pool again getting most of the WETH back
Repay the WETH flashloan
Keep rest of the rewards for oneself

Tools Used

Manual Review

Recommended Mitigation Steps

The issue can be mitigated by using TWAP for the salty pool too, this way the pool can not easily be manipulated using a flashloan.

Assessed type

Oracle

c4-judge commented 7 months ago

Picodes marked the issue as duplicate of #609

c4-judge commented 6 months ago

Picodes marked the issue as satisfactory