sherlock-audit / 2024-02-perpetual-judging

2 stars 2 forks source link

IllIllI - Reservation price does not take into account the size of the order #124

Closed sherlock-admin4 closed 7 months ago

sherlock-admin4 commented 7 months ago

IllIllI

high

Reservation price does not take into account the size of the order

Summary

The reservation price provides its full liquidity at the reservation price, rather than taking into account the order size.

Vulnerability Detail

One of the major risks of the OracleMaker is long/short exposure, and according to the readme, this risk is supposed to be minimal because it attempts to minimize inventory risk by pricing multiple trades in the same direction, with a worse and worse reservation price, creating a disincentive for takers for that side to use the OracleMaker, while allowing trades to occur at the midprice for the opposite direction.

If a user submits 10 long notional orders, each consuming 1/10 of the available OracleMaker liquidity, each order will be given a worse and worse reservation price, which will compensate the OracleMaker for taking on the position risk. If instead, the user submits a single 100 long notional order for the full amount available, and the OracleMaker is currently balanced, the trader will get filled for the full amount at the Pyth oracle's price, and the OracleMaker will be forced to go short the full amount at that price. The attacker would be able to close out their side of the position almost immediately and thereby avoiding funding fees, by using the SpotHedgeBaseMaker.

Impact

The OracleMaker will be forced to take the opposite side of the trade, without getting any price premium. If the OracleMaker is forced to go short while there is a bull-market rally in the market's base token, the OracleMaker (and its LPs) will incur massive losses, and perpetual will become insolvent, because whitelisted makers such as the OracleMaker cannot be liquidated.

The readme also states that ...the maker would stop providing one side of the liquidity, which is undesirable because he won’t be earning fees on that side, so this would also result in a loss of fees, leading to not being able to recoup the costs of being exposed to informed order flow.

Code Snippet

The reservation price offered to the trader is solely based on existing positions, and doesn't take into account the amount being requested:

// File: src/maker/OracleMaker.sol : OracleMaker.fillOrder()   #1

265        function fillOrder(
266            bool isBaseToQuote,
267            bool isExactInput,
268            uint256 amount,
269            bytes calldata
270        ) external onlyClearingHouse returns (uint256, bytes memory) {
271            uint256 basePrice = _getPrice();
272:           uint256 basePriceWithSpread = _getBasePriceWithSpread(basePrice, isBaseToQuote);

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

Tool used

Manual Review

Recommendation

Take into account the amount being asked for, when quoting the spread

Duplicate of #25

sherlock-admin3 commented 7 months ago

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

santipu_ commented:

Medium

takarez commented:

The natsepc says that the liquidation of a whitelisted maker is not possible but they working to implement a safer mechanism for it, making this impossible