c4-bot-3 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/price_feed/PriceAggregator.sol#L141-L143 https://github.com/code-423n4/2024-01-salty/blob/main/src/price_feed/PriceAggregator.sol#L176-L197 https://github.com/code-423n4/2024-01-salty/blob/main/src/price_feed/CoreChainlinkFeed.sol#L14-L23 https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L195-L206

Vulnerability details

Impact

The Chainlink Oracle in PriceAggregator uses BTC/USD and ETH/USD price feeds are to price WBTC and WETH. Both WBTC and WETH are basically bridged assets, and if either the WBTC or WETH bridge is compromised or fails, then WBTC or WETH will depeg and will no longer be equivalent to BTC or ETH. Since only the Chainlink oracle returns prices based on BTC and ETH rather than WBTC and WETH, but the CoreUniswapFeed and CoreSaltyFeed use WBTC and WETH, the price difference between the price sources will be too far apart and _aggregatePrices will return an error as the comment stated. Also, except in the case of a large depeg, the small differences that always exist between BTC and WBTC (as well as ETH and WETH) will still affect the price since we aggregate the prices and take the average, this can allow us to make small arbitrages when borrowing to the detriment of the protocol.

Proof of Concept

PriceAggregator uses 3 price feeds and one is Chainlink.

IPriceFeed public priceFeed1; // CoreUniswapFeed by default
IPriceFeed public priceFeed2; // CoreChainlinkFeed by default
IPriceFeed public priceFeed3; // CoreSaltyFeed by default

CoreChainlinkFeed uses BTC/USD and ETH/USD price feeds.

AggregatorV3Interface immutable public CHAINLINK_BTC_USD;
AggregatorV3Interface immutable public CHAINLINK_ETH_USD;

Tools Used

Manual Review

Recommended Mitigation Steps

As the third price feed, consider Chainlink's alternatives or give prices by taking Chainlink's WBTC/BTC price feed into account to secure the WBTC side.

Assessed type

Oracle

c4-judge commented 5 months ago

Picodes marked the issue as duplicate of #632