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

11 stars 6 forks source link

Absence of autonomous mechanism for `selling collateral assets in the external market in exchange for USDS` will cause undercollateralization during market crashes and will cause USDS to depeg. #905

Open c4-bot-1 opened 7 months ago

c4-bot-1 commented 7 months ago

Lines of code

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

Vulnerability details

Impact

The stablecoin USDS retains its US dollar peg by being overcollateralized. That is true on a bull market. That is also true on a bear market if the liquidation process is faster than the falling prices of the collateral assets.
However during a bear market, there is a scenario where the the price of collateral assets may tank faster than the liquidation process. In this scenario, the total value of the collateral assets of the protocol may end up being lower than the minted / circulating USDS. This will cause undercollateralization and will cause the USDS to depeg.
The depegging will cause the holders of USDS to lose financially. It may cause panic and that will be an existential threat to the protocol.

Proof of Concept

Tools Used

Manual review

Recommended Mitigation Steps

Create a function to sell assets and acquire USDS on external market and just like liquidateUser() and performUpkeep(), reward the users for doing it (calling the function).
If USDS is not available, buy stablecoins USDC and store it for a while to serve as an emergency collateral backing until the market goes back to normal.

Assessed type

Other

c4-judge commented 7 months ago

Picodes marked the issue as primary issue

c4-sponsor commented 6 months ago

othernet-global (sponsor) acknowledged

othernet-global commented 6 months ago

Note: the overcollateralized stablecoin mechanism has been removed from the DEX.

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

othernet-global commented 6 months ago

Note: the overcollateralized stablecoin mechanism has been removed from the DEX.

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

c4-judge commented 6 months ago

Picodes marked the issue as satisfactory

c4-judge commented 6 months ago

Picodes changed the severity to 2 (Med Risk)

c4-judge commented 6 months ago

Picodes marked the issue as selected for report

Picodes commented 6 months ago

Regrouping as duplicates of this issue reports about the fact that the swaps are not atomic so the protocol holds a temporary change risk.

othernet-global commented 6 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