stopthecap - If any stable depegs, oracle will fail, disabling swaps

[sherlock-admin]

stopthecap

high

If any stable depegs, oracle will fail, disabling swaps

Summary

If any stable depegs, oracle will fail, disabling swaps

Vulnerability Detail

When swapping, the price of the asset/stable is fetched from OracleX. After fetching the price, the deviation is checked in the _checkPrice function.

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/d5328421bea80e3b0fd4595e4eb6b732a40e421e/Unitas-Protocol/src/Unitas.sol#L595-L600

If the price of an asset/stable depegs, the following require will fail:

 _require(minPrice <= price && price <= maxPrice, Errors.PRICE_INVALID);

Due to the fail in the deviation, any swapping activity will be disabled by default and transactions will not go through

Impact

Core functionality of the protocol will fail to work if any token they fetch depegs and its price goes outside the bounds.

Code Snippet

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/d5328421bea80e3b0fd4595e4eb6b732a40e421e/Unitas-Protocol/src/Unitas.sol#L440

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/d5328421bea80e3b0fd4595e4eb6b732a40e421e/Unitas-Protocol/src/Unitas.sol#L595-L600

Tool used

Manual Review

Recommendation

Use a secondary oracle when the first one fails and wrap the code in a try catch and store the last fetched price in a variable

[SunXrex]

Sun: minPrice and maxPrice are used as a precautionary measure in case the feeder role is compromised. Additionally, the min and max prices are set to values that are not expected to be reached in the market.

We think shoud be invaild.

Aditya: Hyper change in EMC price is not accepted beyond a limit by the oracle. Oracle will reject the new price and retain the old price. This will ensure swaps happen. Although this may not happen at the true market rate.

[Adityaxrex]

before the price is sent to Oracle, it is processed by oracle feeder. Feeder rejects hyper change to the prices. Instead the feeder will send maximum allowed change per update.

[ctf-sec]

The minPrice and maxPrice are protocol's design choice to add oracle protection. not a valid issue

[0xffff11]

If as @Adityaxrex explains, the feeder "rejects hyper change to the prices. Instead the feeder will send maximum allowed change per update." , I agree invalid. It is pretty much the same as the proposed fixed of providing a valid price (what they are doing under the hood). So in theory they already safeguard for this according to sponsor comments

[0xruhum]

Escalate for 10 USDC

Hyper change in EMC price is not accepted beyond a limit by the oracle. Oracle will reject the new price and retain the old price.

Feeder rejects hyper change to the prices. Instead the feeder will send maximum allowed change per update.

This was not communicated by the protocol team. Wardens assumed that the oracle would at all times try to represent the true price of the token.

Limiting the oracle to certain values is in itself a security issue. Even if USDT loses its peg you want to continue using $1 for its price? If INR loses value against USD you don't update it either? That would break the whole system since you open up tons of arbitrage opportunities with other protocols.

If you have a strict limit on prices, you run into the issue that big movements in the market will leave you with outdated price data since it moves outside of your limits. By setting a broader limit you allow more accurate pricing if these movements happen but don't protect against the feeder being compromised. If an attacker is able to move the price even by 10% they are able to drain the protocol's collateral (move price up -> take loan -> move price down -> repay loan).

These price limits offer no protection against the feeder being compromised. You have to find a different solution for that. For example, instead of having just 1 EOA that's allowed to push price data, you set up multiple ones in different locations and then take the avg price. That would mean that the attacker has to compromise a majority of the EOA to have any real impact on pricing. Instead, you implement a naive price limit that will just cause your system to break whenever any big price movements happen. Just take USDC as an example. A stablecoin that was generally regarded as the safest out there. Even DAI uses it as collateral. Because of the banking crisis in the USA back in March/April, the price dropped below 90 cents. USDT is also full of uncertainty.

To update the price limits you have to go through the timelock. Quick reactions to these unexpected events will be impossible.

If you push price data that's outside the price limit you'll cause swaps to halt. That means users can't repay their loans to take out their collateral. Effectively you freeze their funds. If you don't push price data that's outside the price limit, you force the protocol to work with outdated price data which opens up arbitrage opportunities that will break the protocol. Either way, you have a security issue here.

[sherlock-admin]

Escalate for 10 USDC

Hyper change in EMC price is not accepted beyond a limit by the oracle. Oracle will reject the new price and retain the old price.

Feeder rejects hyper change to the prices. Instead the feeder will send maximum allowed change per update.

This was not communicated by the protocol team. Wardens assumed that the oracle would at all times try to represent the true price of the token.

Limiting the oracle to certain values is in itself a security issue. Even if USDT loses its peg you want to continue using $1 for its price? If INR loses value against USD you don't update it either? That would break the whole system since you open up tons of arbitrage opportunities with other protocols.

If you have a strict limit on prices, you run into the issue that big movements in the market will leave you with outdated price data since it moves outside of your limits. By setting a broader limit you allow more accurate pricing if these movements happen but don't protect against the feeder being compromised. If an attacker is able to move the price even by 10% they are able to drain the protocol's collateral (move price up -> take loan -> move price down -> repay loan).

These price limits offer no protection against the feeder being compromised. You have to find a different solution for that. For example, instead of having just 1 EOA that's allowed to push price data, you set up multiple ones in different locations and then take the avg price. That would mean that the attacker has to compromise a majority of the EOA to have any real impact on pricing. Instead, you implement a naive price limit that will just cause your system to break whenever any big price movements happen. Just take USDC as an example. A stablecoin that was generally regarded as the safest out there. Even DAI uses it as collateral. Because of the banking crisis in the USA back in March/April, the price dropped below 90 cents. USDT is also full of uncertainty.

To update the price limits you have to go through the timelock. Quick reactions to these unexpected events will be impossible.

If you push price data that's outside the price limit you'll cause swaps to halt. That means users can't repay their loans to take out their collateral. Effectively you freeze their funds. If you don't push price data that's outside the price limit, you force the protocol to work with outdated price data which opens up arbitrage opportunities that will break the protocol. Either way, you have a security issue here.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[Adityaxrex]

This issue is valid and accepted. We will need to find a reasonable solution for this. Thank you

[SunXrex]

1. Again, the min and max prices are set to values that are not expected to be reached in the market.

2. I agree with this statement~ However, by doing so, it would be detected by our monitor, and attackers might not use a single transaction to drain the pool. If an attacker is able to move the price even by 10% they are able to drain the protocol's collateral (move price up -> take loan -> move price down -> repay loan).

3. Apologies for not disclosing too much about how we protect our Feeder EOA. We use Shamir's secret sharing, following an M of N scheme, across different containers.

4. Thank you for escalating this. It makes us pay more attention to this issue.

5. It should be Medium.

[jacksanford1]

Generally agree that: 1) Human cannot be relied upon for making this a non-issue 2) The max and min safeguards could create problems on their own (if min price is 95 cents and USDT is trading at 70 cents and people are able to use the 95 cent price for real transactions) 3) Stablecoins can be fairly stable, but they can be very unstable as well and there needs to be a plan for handling extreme instability

Will call this a Medium for now but I could see a case for a High (stablecoins depeg a lot depending on the magnitude of depeg needed to cause an issue). And will need more information from someone in order to downgrade it back to low/invalid.

[hrishibhat]

Result: Medium Has duplicates

[sherlock-admin]

Escalations have been resolved successfully!

Escalation status:

• 0xruhum: accepted

[jacksanford1]

Acknowledged by protocol team (won't fix).