Perp::computeRequiredAmounts applies the calculated offset incorrectly, leading to incorrect amount of tokens traded

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/main/src/libraries/Trade.sol#L45-L47
https://github.com/code-423n4/2024-05-predy/blob/main/src/libraries/Perp.sol#L462-L467
https://github.com/code-423n4/2024-05-predy/blob/main/src/libraries/Trade.sol#L84
https://github.com/code-423n4/2024-05-predy/blob/main/src/libraries/Trade.sol#L94

Vulnerability details

Impact

When trades are executed, Perp::computeRequiredAmounts is called to calculate the required token amounts that will be used in the swap.

This amount is used to determine how much base tokens to trade with the user.

Perp::computeRequiredAmounts applies an offset on the requiredAmounts for better accuracy. As mentioned by the sponsor on discord, "We remove the offset from the Uni LP and convert it to Squart.".

In addition, the Predy docs mention "What we aim to do is to maintain Squart at 2√x by adjusting the offset during rebalances."

The problem is that the sign of the offset and requiredAmounts are not taken into account, the requiredAmounts is always subtracted by the offset.

    requiredAmountUnderlying -= offsetUnderlying;
    requiredAmountStable -= offsetStable;

The intention is that the offset should reduce the requiredAmounts. However, depending on the signs of these values, it may incorrectly increase.

1. (requiredAmount < 0 && offset < 0) => requiredAmount will incorrectly increase
2. (requiredAmount > 0 && offset > 0) => requiredAmount will correctly decrease
3. (requiredAmount < 0 && offset > 0) => requiredAmount will correctly decrease
4. (requiredAmount > 0 && offset < 0) => requiredAmount will incorrectly increase (since we are subtracting with a negative value)

When cases #1 and #4 occur, the requiredAmount is increased when it should be decreased.

This will lead to an incorrect swap value for the amount of base tokens to trade. Users or LPs may suffer due to losing more base tokens on the trade.

Proof of Concept

Trade.sol#L45-L47

    (int256 underlyingAmountForSqrt, int256 stableAmountForSqrt) = Perp.computeRequiredAmounts(
        pairStatus.sqrtAssetStatus, pairStatus.isQuoteZero, openPosition, tradeParams.tradeAmountSqrt
    );

The required token amounts are calculated, which are required for the token swaps for the trade.

Perp.sol#L462-L467

    (int256 offsetUnderlying, int256 offsetStable) = calculateSqrtPerpOffset(
        _userStatus, _sqrtAssetStatus.tickLower, _sqrtAssetStatus.tickUpper, _tradeSqrtAmount, _isQuoteZero
    );

    requiredAmountUnderlying -= offsetUnderlying;
    requiredAmountStable -= offsetStable;

Offsets are applied to the required token amounts to ensure they are not overestimated, which is why they are deducted.

However, the signs of requiredAmountUnderlying, requiredAmountStable, offsetUnderlying, offsetStable can be either positive or negative.

Let's view what happens in these two cases:

1. (requiredAmount > 0 && offset < 0) => requiredAmount -= (-offset) => requiredAmount += offset. Here, requiredAmount is increased
2. (requiredAmount < 0 && offset < 0) => -requiredAmount -= (-offset) => -requiredAmount += offset. requiredAmount is increased.

In these cases, the offset increases the requiredAmount, rather than decreasing it.

The returned requiredAmountUnderlying is used to calculate the totalBaseAmount for the trade (amount of base tokens to swap). The totalBaseAmount will be inflated because the offset had increased the required amounts.

Trade.sol#L84

 int256 totalBaseAmount = swapParams.amountPerp + swapParams.amountSqrtPerp + swapParams.fee; //@audit `requiredAmountUnderlying` = `swapParams.amountSqrtPerp`

Which is used for the settlement callback

Trade.sol#L94

globalData.callSettlementCallback(settlementData, totalBaseAmount);

totalBaseAmount is the amount of base tokens to swap.

During the callback, base tokens are either sold or bought in exchange for quote tokens. Since the amount of base tokens was calculated incorrectly, this swap will trade an incorrect amount of base tokens, and users/LPs may have to give up more base tokens than required, causing a loss for them.

Tools Used

Manual Review

Recommended Mitigation Steps

Apply the offset correctly, depending on the sign of the required tokens and calculated offset, for both underlying and stable amounts:

    (int256 offsetUnderlying, int256 offsetStable) = calculateSqrtPerpOffset(
        _userStatus, _sqrtAssetStatus.tickLower, _sqrtAssetStatus.tickUpper, _tradeSqrtAmount, _isQuoteZero
    );

-   requiredAmountUnderlying -= offsetUnderlying;
-   requiredAmountStable -= offsetStable;
+
+   if((requiredAmountUnderlying < 0 && offsetUnderlying < 0) ||(requiredAmountUnderlying > 0 && offsetUnderlying < 0)){
+       requiredAmountUnderlying += offsetUnderlying;
+   }else{
+       requiredAmountUnderlying -= offsetUnderlying;
+   }
+    
+   if((requiredAmountStable < 0 && offsetStable < 0) ||(requiredAmountStable > 0 && offsetStable < 0)){
+       requiredAmountStable += offsetStable;
+   }else{
+       requiredAmountStable -= offsetStable;
+   }

Assessed type

Math