shogoki - Unitas swap function is vulnerable to Sandwich Attack at oracle price update

[sherlock-admin]

shogoki

high

Unitas swap function is vulnerable to Sandwich Attack at oracle price update

Summary

WHen the oracle price is updates, an attacker can sandwich the update adderss with 2 swap transactions to gain a profit and drain the collateral.

Vulnerability Detail

Inside the swap function of the Unitas contract the getLatestPrice function of the XOracle contract is used to fetch the current price of the USDx token to be swapped to/from. The price has to be updated periodically because the currencies fluctuate against the USD price. A user could use these fluctuations to speculate on prices and gain a profit or loss.

However,as the price for the oracle is updated by a transaction that is publicly visible inside the mempool, a malicious user or attacker can see the new price before it is active. As the oracle price is the only thing, which has influence of the token number to mint/burn on a swap call, an attacker can easily exploit a temporarily appreciation of an USDx token agains the USD1 (US Dollar price). This can be achieved by "sandwiching" the price update transaction with a transaction to swap USD1 into the relevant USDx token first, and swap it back after the price update.

Example: THe price of USD91 (INR) is to be increased from 0.012 to 0.013 Attacker already holds 10,000 of USD1 tokens.

• The Feeder sends the transaction to update the oracle price, and it gets placed in the mempool.
• Attacker sees these transaction, and sends himself 2 transactions.
  1. Swap 10,000 possible mount of USD1 to USD91
  2. Swap all USD91 to USD1
• The attacker sets the gas to ensure that the first tx gets included before the price update, and the second one after the price update.
• The executed Transactions in order will be:
  1. Attacker swaps 10,000 USD1 to USD91 (should receive: 10,000 / 0.012 = 833,333.333)
  2. FEEDER updates oracle price of USD91 to 0.013
  3. Attacker swaps all USD91 tokens to USD1 (should receive: 833,333.333 * 0.013 = 10,833.333)

The attacker just made 833 USD1 profit in these 2 transactions, and can redeem the USD1 tokens for USDT, which will deduct the collateral by this amount.

Impact

Attacker can gain profit and "steal" collateral

Code Snippet

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

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/main/Unitas-Protocol/src/XOracle.sol#L26-L32

Tool used

Manual Review

Recommendation

The 2 way minting/burning mechanism by the orcale price might be dangerous. However to prevent the specific attack vector, maybe the minting can be paused and unpaused before and after the price update. (in separate transactions)

[SunXrex]

Sun & Lucas: To update prices, we can consider using an RPC (Remote Procedure Call) that helps prevent MEV (Miner Extractable Value).

Risk should be medium.

Usually, updates to the exchange rate for stablecoins result in less than a 1~1.6% change. Additionally, we apply a fee for minting and burning. This makes arbitrage difficult. To provide a perspective from the auditors' case, an increase from 0.012 to 0.013 represents an approximate growth of 8.33% which is almost impossible.

Aditya: Price update frequency is much lower for Unitas compared to other defi swaps like Uniswaps. Unitas update frequency could be as low as once per day. Also the expected users/minters of the protocol are low frequency but high ticket size. Chances of users mint request being in the mempool and Unitas price update happening at the same time given are fairly low. Also, unlike Uniswap any new trade does not move the price. The price is only updated at certain periods of the day or once a day.

[ctf-sec]

Can be a valid medium

[0xffff11]

I agree with a medium. Due to the current architecture of having a fee and the stability of the assets, the value it is quite limited.

[hrishibhat]

Considering this a medium issue based on the above comments.

[Shogoki]

Escalate for 10USDC This should be considered as High:

I was thinking for a longer time on the comments and the reasons for this one to be downgraded to Medium. I even created another escalation on #88 referencing this reasoning. However, after i saw this comment

Hi @Shogoki glad to know your thoughts.

This issue is a valid medium, because price is based on currency exchange rates and it is not fair to say the price is stable or not (depends on the currency itself).

When EMC appriciates / depreciates, the sponsor HAVE TO update price or the protocol will not work properly.

i tend to agree with that. Given that it should bot be a lowering factor, that the prices should be stable. Therefore this should be a high, as reported!

———- If this gets accepted, i believe #88 is a valid medium.

However, If judges decide to reject this escalation here, i think #88 should be rated as low, as stated in my other escalation.

[sherlock-admin]

Escalate for 10USDC This should be considered as High:

I was thinking for a longer time on the comments and the reasons for this one to be downgraded to Medium. I even created another escalation on #88 referencing this reasoning. However, after i saw this comment

Hi @Shogoki glad to know your thoughts.

This issue is a valid medium, because price is based on currency exchange rates and it is not fair to say the price is stable or not (depends on the currency itself).

When EMC appriciates / depreciates, the sponsor HAVE TO update price or the protocol will not work properly.

i tend to agree with that. Given that it should bot be a lowering factor, that the prices should be stable. Therefore this should be a high, as reported!

———- If this gets accepted, i believe #88 is a valid medium.

However, If judges decide to reject this escalation here, i think #88 should be rated as low, as stated in my other escalation.

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.

[ShaheenRehman]

Escalate for 10 USDC

I agree with @Shogoki that this should be considered as a High.

I believe It's a High issue even if prices are not gonna updated frequently because It's not only that the attacker can make a profit from that. Tokens holders can prevent loss as well. And not just prevent the loss, they can turn it into profits by exploiting this loophole (I have provided a more detailed explanation of this in my submission #105 ) As the protocol team said "updates to the exchange rate for stablecoins result in less than a 1~1.6% change", I agree but we must also take into account unusual scenarios, particularly when dealing with EMC Markets. Who knows the market? Maybe the prices get updated frequently!

I completely agree with the points @Shogoki has raised, If this issue is not deemed High, then it would be reasonable to classify #88 as a low priority.

Thanks!

[sherlock-admin]

Escalate for 10 USDC

I agree with @Shogoki that this should be considered as a High.

I believe It's a High issue even if prices are not gonna updated frequently because It's not only that the attacker can make a profit from that. Tokens holders can prevent loss as well. And not just prevent the loss, they can turn it into profits by exploiting this loophole (I have provided a more detailed explanation of this in my submission #105 ) As the protocol team said "updates to the exchange rate for stablecoins result in less than a 1~1.6% change", I agree but we must also take into account unusual scenarios, particularly when dealing with EMC Markets. Who knows the market? Maybe the prices get updated frequently!

I completely agree with the points @Shogoki has raised, If this issue is not deemed High, then it would be reasonable to classify #88 as a low priority.

Thanks!

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.

[ctf-sec]

Agree with high

[jacksanford1]

The case for Medium is that the frequency of being able to steal a material amount of funds seems very rare:

• will be used for stablecoins (which can have large price swings, but the frequency of large prices swings is normally low)
• there will be a fee for minting/burning which eats into profitability

The case for High is that the sponsor is:

• not assuming prices are relatively stable in #88
• Sponsor comment in #88 that "sponsor HAVE TO update price or the protocol will not work properly" means that the protocol's hand could be forced in making a large price update even if there's a scenario where it will get sandwiched

I am fine with High unless someone makes a stronger case for Medium.

[SunXrex]

No code change in this fix. We will update the price over Flashbot (private transaction).

[jacksanford1]

Understood. In that case, we'll change the label for this issue to "Won't Fix". cc @SunXrex

[hrishibhat]

Result: High Has duplicates

[sherlock-admin]

Escalations have been resolved successfully!

Escalation status:

• Shogoki: accepted
• DevABDee: accepted

[jacksanford1]

Acknowledged by protocol team (won't fix).