Emmanuel - Users will cheat a Product when its makerFee/takerFee is more than its maintenance

[sherlock-admin]

Emmanuel

medium

Users will cheat a Product when its makerFee/takerFee is more than its maintenance

Summary

Since there is no check in Product#initialize that ensures that maintenance is always greater than makerFee/takerFee, a user would cheat products whose makerFee is higher than its maintenance. The user would use the minCollateral amount of collateral to open a precalculated position such that:

• after debiting the open position fee, account position will pass the maintenanceInvariant check, but will be using the max leverage possible
• the open position fee deducted will be more than half of the user's collateral
• the close position fee deducted(which is equal to open position fee) will wipe out the remaining collateral and increase the product's shortfall.

Vulnerability Detail

Since opening a position and closing a position debits an amount of fee(which are equal) from the user's collateral, a user would craft a position such that the collateral balance after debiting the open position fee would be less than the close position fee. This would add to the Product's debt shortfall. There is a maintenanceInvariant check in openPosition functions that makes sure that debiting the openPosition fee from account's collateral will not make the balance less than the maintenance requirement. Therefore, in order to satisfy maintenanceInvariant, User would use the minCollateral (x) to open a position (y) such that y is the maximum value from this equation: x - (y * positionFee) > y * maintenance

ATTACK SCENARIO

• Product is launched with following configurations:
  • minCollateral=100
  • maintenance=0.25; max leverage=4x
  • takerFee=0.3(higher than maintenance)
• Malicious user Alice calculates the position he should open to maximally cheat the Product using the above inequality:
  • 100 - (y * 0.3) > y * 0.25
  • Solving for y, we get y < 181.8
• So, Alice uses a collateral of 100 USDC to open a taker position of $181 (that is about 1.8x leverage)
• open position takerFee of 0.3 * 181 = 54 is debited from the account
• collateral balance of account is 100-54 = 46 USDC; open taker position is 181; current leverage= 181/46=3.93x
• Alice closes his position which would debit takerFee of 0.3 * 181 = 54 again
• since Alice collateral balance was 46, debiting a fee of 54 will increase shortfall of the product by 54-46=8
• Alice has succeeded in increasing the Product's shortfall, and since Alice has been pardoned(i.e. No outstanding debt is recorded for him), He would repeat the attack again,thereby deliberately increasing the amount of debt the Product owner has to pay. This will also affect other users of the product as they will not be able to withdraw their collateral until Product owner resolves the shortfall.

Although it is unlikely(not impossible) that Protocol owned products will be well engineered and not susceptible to this, other individuals or organizations that may want to launch their own products must be protected from this vulnerability.

Impact

A malicious user will deliberately increase a Product's shortfall; force Product owner to resolve the debt; Prevent other users of the product from withdrawing their collateral until Product owner resolves the shortfall.

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/product/Product.sol#L224-L227 https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/product/Product.sol#L265-L268

Tool used

Manual Review

Recommendation

Consider implementing any of these:

• In Product#initialize function, include a check that prevents users from creating a product whose makerFee or takerFee is more than its maintenance
• The maintenanceInvariant check should make sure that the account should have enough margin for next settlement, even after debiting the closePosition fee.

[arjun-io]

This is an interesting case, but since the attacker does not profit at all from the griefing (and instead, as in the example suffers large losses) we think this is a low severity.

[Emedudu]

Escalate for 10 USDC

Please, consider the following:

• This attack may or may not be deliberate (A user may innocently decide to use ~minCollateral to open a position at ~optimal leverage for the attack)
• Attacker needs only minCollateral of collateral token to perform this attack, and considering the range of values minCollateral could be, performing this attack is not really expensive. I used 100 USDC as an example, but minCollateral could actually be less. In a previous deployment of Controller on Arbitrum, we can see that minCollateral was set to ~$10.
• Product owner would be forced to repay abnormally created debt.
• Other users of the Product will be affected: They may not be able to withdraw all their collateral, and they may escape some liquidations because debitAccount() could revert.
• Attacker will be completely pardoned, so he can replay attack after next settlement regardless of if the shortfall has been resolved or not.
• Essentially, by risking minCollateral, attacker would deny other users from withdrawing their collateral, allow other users to escape liquidations, and force product owner to repay debt he created.
• There is an easy fix that can totally prevent this.

[sherlock-admin]

Escalate for 10 USDC

Please, consider the following:

• This attack may or may not be deliberate (A user may innocently decide to use ~minCollateral to open a position at ~optimal leverage for the attack)
• Attacker needs only minCollateral of collateral token to perform this attack, and considering the range of values minCollateral could be, performing this attack is not really expensive. I used 100 USDC as an example, but minCollateral could actually be less. In a previous deployment of Controller on Arbitrum, we can see that minCollateral was set to ~$10.
• Product owner would be forced to repay abnormally created debt.
• Other users of the Product will be affected: They may not be able to withdraw all their collateral, and they may escape some liquidations because debitAccount() could revert.
• Attacker will be completely pardoned, so he can replay attack after next settlement regardless of if the shortfall has been resolved or not.
• Essentially, by risking minCollateral, attacker would deny other users from withdrawing their collateral, allow other users to escape liquidations, and force product owner to repay debt he created.
• There is an easy fix that can totally prevent this.

You've created a valid escalation for 10 USDC!

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.

[KenzoAgada]

I think it's good to point this out but I'm not sure this is enough for a medium in Sherlock standards. By the way I was thinking more that this might happen accidently and not on purpose. (As the issue and sponsor point out, attacker will lose by doing this.)

[jacksanford1]

Yes, need either: 1) Way to show that this is profitable for an attacker or 2) Way to show that this could realistically occur accidentally

And even then, I'm not sure why the impact is severe enough? Temporary freezing? @Emedudu

[jacksanford1]

Result: Invalid Unique Invalid based on Arjun's comment and lack of update from escalator.

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• Emedudu: rejected