PUSH0 - OracleMaker's price with spread does not take into account the new position

[sherlock-admin4]

PUSH0

high

OracleMaker's price with spread does not take into account the new position

Summary

OracleMaker's _getBasePriceWithSpread() does not take into account the opening position's size, but only on the current position of the Maker.

This means there is no price impact protection against large trades (or any trades at all) for the Oracle Maker. Anyone can then bypass the spread by opening a reverse position before actually opening their intended position.

Vulnerability Detail

To reduce risky positions, the Oracle Maker will quote a slightly worse price (for the trader) than the actual Oracle price for any positions that increases risk. This is also mentioned in the audit docs, Dynamic Premium section.

When a new position is requested, the Oracle Maker quotes a price that includes this spread:

function fillOrder(
    bool isBaseToQuote,
    bool isExactInput,
    uint256 amount,
    bytes calldata
) external onlyClearingHouse returns (uint256, bytes memory) {
    uint256 basePrice = _getPrice();
    uint256 basePriceWithSpread = _getBasePriceWithSpread(basePrice, isBaseToQuote); // @audit here

There is no spread when the new position reduces risk (e.g. if the current Maker position is +3 ETH, and the new order implies -1 ETH, then the Maker will quote the oracle price directly).

However, _getBasePriceWithSpread() never uses the order's amount, therefore large positions will be quoted the same price as small positions, i.e. there is no price impact. This issue also exists if the new position passes the zero mark, where the Maker thinks it's de-risking, while in reality it's being subject to much more risk in the opposite direction.

• Suppose the Oracle Maker's current total position is 1 ETH long (+1 ETH)
• Someone opens a 100 ETH long position, the Oracle Maker thinks it's de-risking by being able to open a 100 ETH short, and quotes the oracle price.
• After the trade, the Maker is actually in a much riskier position of 99 ETH short (-99 ETH)

Anyone can then bypass the spread completely (or partially) by opening a position in the opposite direction before the intended direction:

• The Oracle Maker's current total position is 1 ETH long (+1 ETH).
• Alice wants to open a 10 ETH long (+10 ETH) position.

If Alice sends a 10 ETH long order now, she would have to accept the base price spread and get a slightly worse price than the Pyth oracle price. However Alice can bypass the spread by sending the following two orders in quick succession to the relayer:

• First order: 1 ETH short (-1 ETH)
  • The Maker thinks it's de-risking, so it quotes the oracle price directly.
  • After this order, the Maker has 0 ETH position.
• Second order: 11 ETH long (+11 ETH)
  • Since the Maker's position is zero, it quotes the oracle price.
  • Alice has opened a net total of +10 ETH as intended. However she is not subject to the price spread.

Alice was able to bypass the premium from the spread model, and force the Maker to quote exactly the Pyth oracle price. Note that Alice doesn't have to fully bypass the spread, she could have just opened a -0.5 ETH first and would still largely bypass the spread already.

Impact

• Dynamic Premiums from the price spread can be bypassed completely, traders can always be quoted the oracle price without spread (or with a heavy reduction of the spread).
• Maker is exposed to more risk than the intended design.

Code Snippet

https://github.com/sherlock-audit/2024-02-perpetual/blob/main/perp-contract-v3/src/maker/OracleMaker.sol#L272

https://github.com/sherlock-audit/2024-02-perpetual/blob/main/perp-contract-v3/src/maker/OracleMaker.sol#L401-L409

Tool used

Manual review

Recommendation

_getBasePriceWithSpread() must take into account the average spread of the Maker's position before and after the trade, not just the position before the trade.

• See proof here. Note this formula still works when the new position movement crosses the zero mark, as the integral of a constant zero function is zero.

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

santipu_ commented:

Medium

[rc56]

@lnxrp:

• Won't fix
• It is true that the trick (reverse position before actually opening their intended position) could bypass OracleMaker's spread, but the impact is considered a degradation (high risk exposure without proper compensation) of maker performance instead of a critical vulnerability
• Note the maker has minMarginRatio which protects it from taking too much exposure. The parameter had been set conservative (minMarginRatio = 100%) from the start so we have extra safety margin to observe its real-world performance and improve it iteratively

[midori-fuse]

Escalate

We have shown the method to bypass the Oracle Maker's spread, which is meant to be the protection against large exposures. This attack also has no external conditions: With any amount of margin, you can still partially bypass the spread. Larger margin only maximizes the impact, but smaller margin does not prevent it in any way.

The effect of bypassing the premium is that the Oracle Maker is forced to take positions without spread (or with a heavy reduction). Elaborating on this, this means that the Oracle Maker is forced to take the same position size for a larger (absolute) open notional. This translates to a direct loss should the price moves in any direction:

• If the price goes in the favor of the Oracle Maker's position, the larger open notional decreases the Maker's profit.
• If the price goes against the favor of the Oracle Maker's position, the larger open notional increases the Maker's loss.

Note that we are only elaborating what is already written in our report (elaborating the direct effect of the Maker quoting a less favorable price). The elaborated info only arises from the definition of a standard perpetual contract, and we are adding no additional info.

Therefore the impact translates to direct loss under any resulting price movements. Because it also has no external conditions for this trick to be possible, I believe this fits into the criteria of a High risk issue.

[sherlock-admin2]

Escalate

We have shown the method to bypass the Oracle Maker's spread, which is meant to be the protection against large exposures. This attack also has no external conditions: With any amount of margin, you can still partially bypass the spread. Larger margin only maximizes the impact, but smaller margin does not prevent it in any way.

The effect of bypassing the premium is that the Oracle Maker is forced to take positions without spread (or with a heavy reduction). Elaborating on this, this means that the Oracle Maker is forced to take the same position size for a larger (absolute) open notional. This translates to a direct loss should the price moves in any direction:

• If the price goes in the favor of the Oracle Maker's position, the larger open notional decreases the Maker's profit.
• If the price goes against the favor of the Oracle Maker's position, the larger open notional increases the Maker's loss.

Note that we are only elaborating what is already written in our report (elaborating the direct effect of the Maker quoting a less favorable price). The elaborated info only arises from the definition of a standard perpetual contract, and we are adding no additional info.

Therefore the impact translates to direct loss under any resulting price movements. Because it also has no external conditions for this trick to be possible, I believe this fits into the criteria of a High risk issue.

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.

[nirohgo]

Escalate

This is low/informational

The attack described can not be performed as described. This is because traders can only open positions with the Oracle Maker through the relayer. This means there is no way to know which other orders will be settled before or between the two attack steps (or even their order of execution). The Oracle Maker position direction can change direction in the meantime, making the attacker get a worse price instead of better. Even if the Oracle Maker enabled direct interaction there would still be the risk of other transactions getting in before the attacker and changing the OM position direction.

Without the attack, this is a design improvement and not a medium severity finding.

[sherlock-admin2]

Escalate

This is low/informational

The attack described can not be performed as described. This is because traders can only open positions with the Oracle Maker through the relayer. This means there is no way to know which other orders will be settled before or between the two attack steps (or even their order of execution). The Oracle Maker position direction can change direction in the meantime, making the attacker get a worse price instead of better. Even if the Oracle Maker enabled direct interaction there would still be the risk of other transactions getting in before the attacker and changing the OM position direction.

Without the attack, this is a design improvement and not a medium severity finding.

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.

[Evert0x]

@nevillehuang what's your response to these escalations?

From my understanding, is it possible to execute these two orders in the same block/tx? Or does it depend on external transaction (oracle price update) between the two orders?

[midori-fuse]

According to the contest README:

The relayer may have some value extraction strategies available (for example by delaying the execution of the orders), and is trusted not to use them.

While it is not possible to execute two orders in the same tx, opening two orders in quick succession should still not be difficult (you just literally make actions quickly no?). If the relayer is trusted to resolve orders without utilizing any value-extraction strategies, then it is trusted to resolve orders in a timely manner, in the order it received.

Relayer should update the oracle price before each order anyway (it is stated that the protocol will use Multicaller, so it should be assumed that the Oracle Maker's price at the time of order is always fresh). However if you open orders quickly (in, say, a few seconds, or even less than a second if you can make two mouse clicks quickly), then the oracle price change should not be substantially large, and the chances of any order getting in between is extremely low (not mentioning changing maker position directions as a whole).

[nirohgo]

The arrival of orders sent to an offchain relayer depends on networking, so its not even guaranteed that your orders will get to the relayer at the order you sent them, let alone other orders getting before/in between your orders. If trade volume is high it's even likely. The point is the attacker will be taking a risk that I don't think is justified by the potential gain.

[midori-fuse]

Network related factors are dependent on the admin and external admin.

• Because admins are trusted, whichever endpoint that is used to relay orders are also trusted to be live and relaying the correct information.
• If your own network has a problem, then it's your problem, and completely out of scope.

[nevillehuang]

@midori-fuse Given this constraint highlighted here, I believe medium severity is appropriate. Do you agree?

[midori-fuse]

No, unfortunately I don't agree that the constraint makes sense.

First of all, as I have stated, network congestion related factors are related to the external admin and your own network, which has to be assumed to be trusted to provide reliable service.

Second of all, even given that "constraint", you can open orders, say, two seconds apart, for a completely negligible oracle price difference with the same bypassing effect, and the risk that the price moves sharply within those two seconds is epsilon.

[nirohgo]

To clarify, this in not due to a problem in network congestion, or a problem with the internet on any side (sender or receiver). It's just the way that TCP/IP works. Two messages leaving from point A to B at virtually the same time can and often do take entirely different routes and arrive at different times (and order). Also, because any random order may shift the position direction, orders that interfere include orders that left well before the attacker's.

[midori-fuse]

Even so, it does not change the fact that two orders can still be made one after another, in a reliable fashion by introducing a very small delay between them.

And even without the attack, we have shown that large trades has no price impact compared to smaller trades that makes up the same sum. Because this results in both:

• Exposure to higher risk against the same price sum, equating to a guaranteed loss.
• Enables a path that bypasses the spread directly, forcing the maker to quote a higher price than design. The mentioned risk can be controlled by choosing the appropriate delay between orders, and literally just by having a stable connection.
  • Furthermore the Maker has to actually change position from negative to positive and vice-versa in the meantime, which we believe is too high of an assumption to be considered an attack risk.

we still believe this is a High risk issue.

[Czar102]

I think this report lies on the Med/High borderline, but closer to the Medium severity issue – the loss is only the loss of fees, the attacker has no way of predicting the price anyway.

I don't think the point in @nirohgo's escalation weights a lot into this judgment.

Planning to reject both escalations and leave the issue as is.

[midori-fuse]

It is indeed true that the loss is only a loss of fees (even if it can be bypassed completely).

However the loss here is directed towards Oracle Maker LPs and not the protocol. Furthermore, the issue isn't about a price-based attack, it's a general trick/exploit path to open better positions than the design for traders. The higher the Maker's position, the higher the fee loss, since the premium there is also larger.

So I still think it's a High, since I see loss of fees towards LPs equates to loss of funds.

However I do believe the context on the issue is clear now, and it's a matter of judgement instead of on further analyzing the issue.

[WangSecurity]

Agree with what Czar said above, the only loss are fees and the attack depends on external factors to be true, since the caller cannot control if their transactions will be executed one right after another.

Planning to reject the escalation and leave the issue as it is.

[Evert0x]

Result: Medium Has Duplicates

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• midori-fuse: rejected
• nirohgo: rejected

[nirohgo]

@Evert0x my escalation should not be rejected here because it affected the decision on the original escalation ("the only loss are fees and the attack depends on external factors to be true, since the caller cannot control if their transactions will be executed one right after another.") my escalation demonstrated that the caller cannot control if their transactions will be executed one right after another.