Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xiaoming90 - Position's leverage factor can exceed the protocol's maximum allowable leverage #49

Open
sherlock-admin3 opened this issue Oct 8, 2024 · 0 comments

Comments

@sherlock-admin3
Copy link

sherlock-admin3 commented Oct 8, 2024

xiaoming90

High

Position's leverage factor can exceed the protocol's maximum allowable leverage

Summary

The position's leverage factor can exceed the protocol's maximum allowable leverage, thus breaking this important invariant and increasing the risk of the protocol's insolvency.

Root Cause

  • Position's leverage factor was not validated again when the price was updated during PnL settlement within the settleUpnl function

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

When opening a new position, the protocol will ensure that the position's leverage does not exceed the maximum leverage configured by the protocol at Line 169 below. This invariant must be upheld to prevent over-leveraging, which could potentially lead to the protocol's insolvency.

https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/libraries/LibPartyBPositionsActions.sol#L169

File: LibPartyBPositionsActions.sol
36: 	function openPosition(uint256 quoteId, uint256 filledAmount, uint256 openedPrice) internal returns (uint256 currentId) {
37: 		QuoteStorage.Layout storage quoteLayout = QuoteStorage.layout();
..SNIP..
167: 		// check leverage (is in 18 decimals)
168: 		require(
169: 			(quote.quantity * quote.openedPrice) / quote.lockedValues.totalForPartyA() <= SymbolStorage.layout().symbols[quote.symbolId].maxLeverage,
170: 			"PartyBFacet: Leverage is high"
171: 		);

The leverage of a position is computed based on the following formula.

leverage = (quote.quantity * quote.openedPrice) / quote.lockedValues.totalForPartyA()

Looking at the above equation, if the quote.openPrice increases, the leverage will increase too.

Assume the simplified example where Bob (PartyA) opens the following quote:

  • Long position

  • Symbol is ETH

  • quote.openPrice is 3000 USDC

  • quote.quantity is 1

  • quote.lockedValues.totalForPartyA() is 100 USD, which means that a value of 100 USD is locked within the quote.

In this case, the leverage is 30x. Assume that the protocol's maxLeverage is 30x. Thus, this position's leverage is acceptable.

leverage = (quote.quantity * quote.openedPrice) / quote.lockedValues.totalForPartyA()
leverage = (1 * 3000 USD) / 100 USD = 30x

After a period of time, the current price of ETH becomes 4000 USD. Thus, this quote's PNL is +1000 USD. Assume that one of the PartyB wants to settle 500 USD out of 1000 USD profit from this quote. Thus, PartyB will set the updatedPrices to 3500 USD, which will in turn update the quote.openedPrice from USD 3000 to 3500 USD at Line 77 below. The validation check at Lines 57-67 will pass, and 500 USD will be credited to Bob's allocated balance at Line 69 below.

https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/libraries/LibSettlement.sol#L57

File: LibSettlement.sol
15: 	function settleUpnl(
16: 		SettlementSig memory settleSig,
17: 		uint256[] memory updatedPrices,
18: 		address partyA,
19: 		bool isForceClose
20: 	) internal returns (uint256[] memory newPartyBsAllocatedBalances) {
..SNIP..
57: 			if (quote.openedPrice > data.currentPrice) {
58: 				require(
59: 					updatedPrices[i] < quote.openedPrice && updatedPrices[i] >= data.currentPrice,
60: 					"LibSettlement: Updated price is out of range"
61: 				);
62: 			} else {
63: 				require(
64: 					updatedPrices[i] > quote.openedPrice && updatedPrices[i] <= data.currentPrice,
65: 					"LibSettlement: Updated price is out of range"
66: 				);
67: 			}
68: 			if (quote.positionType == PositionType.LONG) {
69: 				settleAmounts[data.partyBUpnlIndex] +=
70: 					((int256(updatedPrices[i]) - int256(quote.openedPrice)) * int256(LibQuote.quoteOpenAmount(quote))) /
71: 					1e18;
72: 			} else {
73: 				settleAmounts[data.partyBUpnlIndex] +=
74: 					((int256(quote.openedPrice) - int256(updatedPrices[i])) * int256(LibQuote.quoteOpenAmount(quote))) /
75: 					1e18;
76: 			}
77: 			quote.openedPrice = updatedPrices[i];
78: 		}

Let's verify if the leverage of the updated quote/position exceeds the protocol's maximum leverage of 30x. The following shows that the new quote exceeds the maximum leverage, thus breaking the protocol's invarient.

leverage = (quote.quantity * quote.openedPrice) / quote.lockedValues.totalForPartyA()
leverage = (1 * 3500 USD) / 100 USD = 35x

Impact

The purpose of having maximum leverage is to avoid over-leveraging, which could potentially lead to the protocol's insolvency. Thus, this invariant must be upheld. However, the above example shows that the position's leverage factor can exceed the protocol's maximum allowable leverage, thus breaking this important invariant and increasing the risk of the protocol's insolvency.

PoC

No response

Mitigation

Implement additional validation to ensure that the updated quotes do not exceed the maxLeverage within the settleUpnl function.

@sherlock-admin3 sherlock-admin3 changed the title Careful Fiery Scallop - Position's leverage factor can exceed the protocol's maximum allowable leverage xiaoming90 - Position's leverage factor can exceed the protocol's maximum allowable leverage Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant