mstpr-brainbot - Oracle price updates can be easily sandwiched for atomic profits

[sherlock-admin4]

mstpr-brainbot

high

Oracle price updates can be easily sandwiched for atomic profits

Summary

When the new price is posted by admin to oracle, any MEV searcher can frontrun/sandwich for atomic profits. Such sandwich would mean loss of funds for the supplier of the tokens.

Vulnerability Detail

Assume USDC is quote token and WOO is base token. WOO does not have a chainlink price feed which means the pricing of WOO will be handled by the WooOracle solely.

Also assume that the WOO price is 0.5$. However, in off-chain markets, the price of WOO increases to 0.55$ and the oracle is updated by the admin so the oracle update will make the onchain price 0.5$ -> 0.55$.

MEV bot sees the oracle price update tx in mempool, and quickly buys WOO tokens from the pool contract and then sells it for more USDC after the oracle update is completed. At the end, MEV bot achieved to buy WOO tokens from 0.5$ and managed to sell all for 0.55$ in 1 tx. The profit that the MEV bot made is basically a loss to the provider of the WOO tokens in the pool since the WOO tokens are sold to a price that is cheaper than it should be.

Coded PoC:

function test_frontrunOraclePriceUpdate() public {
        uint usdcAmount = 1_000_000 * 1e6;
        uint wooAmount = 100_000 * 1e18;
        deal(USDC, ADMIN, usdcAmount);
        deal(WOO, ADMIN, wooAmount);

        vm.startPrank(ADMIN);
        IERC20(USDC).approve(address(pool), type(uint256).max);
        IERC20(WOO).approve(address(pool), type(uint256).max);
        pool.depositAll(USDC);
        pool.depositAll(WOO);
        vm.stopPrank();

        assertEq(IERC20(USDC).balanceOf(address(pool)), usdcAmount);

        uint usdcAmountForTapir = 10_000 * 1e6;
        vm.startPrank(TAPIR);
        deal(USDC, TAPIR, usdcAmountForTapir);
        IERC20(USDC).approve(address(router), type(uint256).max);
        IERC20(WOO).approve(address(router), type(uint256).max);
        vm.stopPrank();

        // sell USDC for WOO before price update, frontrun, initial price is 0.5
        vm.prank(TAPIR);
        uint receivedWOO = router.swap(USDC, WOO, usdcAmountForTapir, 0, payable(TAPIR), TAPIR);
        console.log("Received WOO", receivedWOO);

        // new price is updated, 0.55
        vm.prank(ADMIN);
        oracle.postPrice(WOO, 0.55 * 1e8);

        // immediately sell back 
        vm.prank(TAPIR);
        uint receivedUSDC = router.swap(WOO, USDC, receivedWOO, 0, payable(TAPIR), TAPIR);
        console.log("Received USDC", receivedUSDC);

        // atomic profit
        assertGe(receivedUSDC, usdcAmountForTapir);
    }

Impact

Code Snippet

https://github.com/sherlock-audit/2024-03-woofi-swap/blob/65185691c91541e33f84b77d4c6290182f137092/WooPoolV2/contracts/wooracle/WooracleV2_2.sol#L148-L156
https://github.com/sherlock-audit/2024-03-woofi-swap/blob/65185691c91541e33f84b77d4c6290182f137092/WooPoolV2/contracts/WooPPV2.sol#L152-L170

Tool used

Manual Review

Recommendation

Add a buffer that whenever the price is updated the buffer amount of time has to be passed.
If the oracle updates at t=0, and buffer is 2seconds then the next swap can happen in t=2 to make sure sandwiching is not possible for MEV bots

[fb-alexcq]

• wooracle price update is high-frequent
• the newly posted k, spread and swap fee, will make the sandwich swap less or non profitable in most cases.

[Banditx0x]

Escalate.

I thought of submitting a similar vulnerability but decided against it because it isn't really valid. We had both submitted a similar vulnerability in another contest when it was indeed valid for different reasons. The profit from the POC comes from the fact that the price of the AMM is deviated from the real price of the Oracle prior to the price update.

The root cause of the profit is actually the AMM being deviated from the correct WOO price rather than the oracle update.

The same amount of profit could be made without any oracle price update by simply buying the WOO token from the pool and selling to the offchain market without the AMM being updated.

Here is another way to think about it:

The admin gives accurate oracle updates. The oracle always "corrects" the price, always setting it to a more accurate price.

Does the oracle update ever make the AMM more arbitragable? No.

Did the update increase the arbitrage loss of the AMM? Again no. Any user could have bought the Woo in the pool when it was at the wrong price. However, this is always true of AMM's - if the price of the AMM is different from the "correct/off-chain" price, there is always an arbitrage that can be made. The arbitrage itself is what updates AMM's to correspond to the real exchange rate of 2 assets.

[sherlock-admin2]

Escalate.

I thought of submitting a similar vulnerability but decided against it because it isn't really valid. We had both submitted a similar vulnerability in another contest when it was indeed valid for different reasons. The profit from the POC comes from the fact that the price of the AMM is deviated from the real price of the Oracle prior to the price update.

The root cause of the profit is actually the AMM being deviated from the correct WOO price rather than the oracle update.

The same amount of profit could be made without any oracle price update by simply buying the WOO token from the pool and selling to the offchain market without the AMM being updated.

Here is another way to think about it:

The admin gives accurate oracle updates. The oracle always "corrects" the price, always setting it to a more accurate price.

Does the oracle update ever make the AMM more arbitragable? No.

Did the update increase the arbitrage loss of the AMM? Again no. Any user could have bought the Woo in the pool when it was at the wrong price. However, this is always true of AMM's - if the price of the AMM is different from the "correct/off-chain" price, there is always an arbitrage that can be made. The arbitrage itself is what updates AMM's to correspond to the real exchange rate of 2 assets.

You've created a valid escalation!

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.

[mstpr]

@Banditx0x

WOO's AMM is directly priced by an oracle. It is not comparable to other AMMs like Curve/Balancer/Uniswap, where price changes occur within the pool itself and external factors are not considered.

The WOO oracle updates the pool so that the price of WOO is set at $1. This means that all WOO sells/buys should respect this price. If someone frontruns the oracle update and makes a profit, it wouldn't be considered arbitrage. Instead, it would be considered an attack because the attacker is well aware of the price update and takes advantage, resulting in WOO tokens being sold at a price cheaper than intended, leading to a loss of funds for the protocol.

This is not "arbitrage" at all. The attacker sees the price changes and performs a swap before and after sandwiching the oracle update in the same pool. There is no arbitrage involved, only a pure MEV attack.

Regarding your statement: "However, the same amount of profit could be made without any oracle price update by simply buying the WOO token from the pool and selling to the offchain market prior to the AMM being updated." If you know "prior to the AMM being updated," you are already front-running it. You buy the tokens from the pool and sell them to the off-chain market.

It's important to note that you are comparing the WOO AMM with other AMMs like Curve/Balancer/Uniswap. For example, if the ETH price increases in a Balancer pool, someone can buy ETH in Curve and sell it to the Balancer pool. This would be considered arbitrage, and these AMMs depend on this type of activity because they don't have an "oracle" that tells them the price to adjust their AMM module. However, in WOO, the price occurs off-chain always. The AMM itself does not determine the price as the main truth. Additionally, the WOO pool can't be the primary market because it gets the price from off-chain sources.

Consider another example: Suppose the WOO token price was $1 and now it's $1.2 in the off-chain market, and the oracle updates accordingly. Everyone will buy WOO at $1 and sell it at $1.2 immediately, making a profit and causing a loss for WOO because the token shouldn't be intended to be sold at $1, but rather at $1.2.

Regarding the issue in the Dodo contest, it's rewarded as "medium." The reason it's considered "medium" and not "high" is that the "I" value in Dodo is only set by the admin and is not frequently meant to be updated. However, the WOO oracle updates the price every 2 minutes, so every 2 minutes there is an opportunity for easy profit for any MEV researcher, especially since gas costs are cheap due to the deployment of contracts on networks with low gas fees.

[Banditx0x]

The price is updated after every swap just like Uniswap:

https://github.com/sherlock-audit/2024-03-woofi-swap/blob/65185691c91541e33f84b77d4c6290182f137092/WooPoolV2/contracts/WooPPV2.sol#L618

Just like Uniswap, the MEV opportunities come from price drift between the real exchange rate of assets and the AMM exchange rate. Each oracle update infact reduces MEV opportunities rather than creates new ones.

[mstpr]

The price is updated after every swap just like Uniswap:

https://github.com/sherlock-audit/2024-03-woofi-swap/blob/65185691c91541e33f84b77d4c6290182f137092/WooPoolV2/contracts/WooPPV2.sol#L618

Just like Uniswap, the MEV opportunities come from price drift between the real exchange rate of assets and the AMM exchange rate. Each oracle update infact reduces MEV opportunities rather than creates new ones.

Not really, the price is updated only temporarily till the next oracle update comes along. The prices never occur in the pool, all dependent on the oracle.

Uniswap AMM design is fully dependent on arbitrages and price occurs naturally internally. Woo on the other hand must need to get the price externally, if anyone can sandwich oracle update using the same pool, that wouldn't be arbitrage. Aside from that, Woo pool does not need arbitrageurs to maintain price since the price does not occurs in pool and it is not related with the pools holding.

[DevHals]

I believe that this issue should be invalid as it can be mitigated by pausing the WooPPV2 if the price is going to be updated by the admin, and the implemented pausability functionality is a mitigation as it is mentioned implicitly as a recommendation by this issue:

Recommendation

Add a buffer that whenever the price is updated the buffer amount of time has to be passed.
If the oracle updates at t=0, and buffer is 2seconds then the next swap can happen in t=2 to make sure sandwiching is not possible for MEV bots

[Banditx0x]

fwiw I don't think the suggestion to pause during oracle updates is viable as the updates are frequent according to the sponsor

[mstpr]

The recommendation is not about pausing at all. It is about giving a buffer time. Please read it again.

[DevHals]

The recommendation is not about pausing at all. It is about giving a buffer time. Please read it again.

can't see any difference!
preventing swaps before a buffer time passes and pausing the swaps achieve a similar outcome of disallowing swap transactions for a certain period, in both cases, swap transactions attempted during the restricted period should revert!

[WangSecurity]

I don't have anything to add here and I don't think anything can be added here. I believe Tapir is right here (I know my input is basically nothing here, but I don't see anything that I can add here).

[mstpr]

The recommendation is not about pausing at all. It is about giving a buffer time. Please read it again.

can't see any difference! preventing swaps before a buffer time passes and pausing the swaps achieve a similar outcome of disallowing swap transactions for a certain period, in both cases, swap transactions attempted during the restricted period should revert!

Pausing would not be realistic since it requires the privileged admin role to call the function every 2 min. Buffer on the other hand is just there to ensure no1 takes advantage of the price change in the same block or the next 2 blocks.

[Czar102]

@Banditx0x @mstpr This sentence is true:

Each oracle update infact reduces MEV opportunities rather than creates new ones.

But, the existence of MEV only within this single AMM, due to price updates, as noted in the submission, indeed presents a valid concern. Not because MEV is possible in between this AMM and the whole market, but because the trades can be made in a way that will strictly (in all tokens' balance) decrease the balance of the depositors.

However, this discrepancy of real price <> oracle price, or past oracle price <> new oracle price is assumed to be low by the model, and the fees should coder it, so without market manipulation (when the attacker knows the future direction of the price and hence the oracle), this is not exploitable (the expected value of the attack is negative).

This is indeed an important consideration, but it rather concerns the design to use the oracle – whenever the reserve curve of the AMM is updated, no matter if due to a parameter or oracle update (and except for some mathematically delicate cases where the liquidity is always shifted away from the current price, but these reserve curves become more degenerate with every update), this will be an issue.

Because it's the design, and it should be assumed that fees make up for potential MEV due to oracle updates, I'm planning to consider this issue informational.

[mstpr]

@Banditx0x @mstpr This sentence is true:

Each oracle update infact reduces MEV opportunities rather than creates new ones.

But, the existence of MEV only within this single AMM, due to price updates, as noted in the submission, indeed presents a valid concern. Not because MEV is possible in between this AMM and the whole market, but because the trades can be made in a way that will strictly (in all tokens' balance) decrease the balance of the depositors.

However, this discrepancy of real price <> oracle price, or past oracle price <> new oracle price is assumed to be low by the model, and the fees should coder it, so without market manipulation (when the attacker knows the future direction of the price and hence the oracle), this is not exploitable (the expected value of the attack is negative).

This is indeed an important consideration, but it rather concerns the design to use the oracle – whenever the reserve curve of the AMM is updated, no matter if due to a parameter or oracle update (and except for some mathematically delicate cases where the liquidity is always shifted away from the current price, but these reserve curves become more degenerate with every update), this will be an issue.

Because it's the design, and it should be assumed that fees make up for potential MEV due to oracle updates, I'm planning to consider this issue informational.

I agree. However, the Woo AMM also does not care about the "amount" in swaps. So even a price change of 0.001% can be a profitable trade because the attacker can swap the most the pool has and swap it back.

Also, this is a core contract logic mistake in my opinion. An oracle update indicates that the tokens will be priced at that, but someone front-running and using the previous price, which is always a problem for Woo.
Also, we can't assume the market and assume the price diffs will be small. What if the token crashes in value and the price change is +-10%? Then the profit from performing this attack is a pure loss to Woofi. As a MEV researcher, maybe you won't be sandwiching all the oracle updates due to price differences, but if the price goes off/on at some point in a day or a week, you can extract a very good amount of profit, and this profit will be the loss of Woofi since the tokens are not intended to be swapped at the previous price.

[Czar102]

So even a price change of 0.001% can be a profitable trade because the attacker can swap the most the pool has and swap it back.

This is false because there are fees, right?

What if the token crashes in value and the price change is +-10%?

The tokens are whitelisted because they mustn't behave in that way for WOOFi to be safe for depositors. This is indeed an important consideration, and follows directly from the design. The recommendation doesn't fix the issue at all, one can still buy, wait the buffer period, then sell. This won't be atomic in a block, but doesn't change the exploitability too much.

[Banditx0x]

Aside from fees there's also a oracle.spread which mimics the spread of a orderbook exchange (a gap around the current trading price where there is no liquidity), which means that the price you get is always "worsened" by the spread amount. Every swap you are effectively charged spread+fees

[Czar102]

Result:
Low
Unique

[sherlock-admin3]

Escalations have been resolved successfully!

Escalation status:

• banditx0x: accepted