cergyk - A malicious user can create a maker position to imbalance vault

[sherlock-admin]

cergyk

high

A malicious user can create a maker position to imbalance vault

Summary

A malicious user can create a maker position to prevent vault from correctly rebalancing

Vulnerability Detail

We can see that during rebalancing, the balanced vault handles the makerLimit() gracefully by creating the biggest maker position possible on any product: https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-vaults/contracts/balanced/BalancedVault.sol#L515

However, any user can take advantage of this: before a deposit (when the vault should increase exposure to underlying markets), take a maker position on an underlying market to push maker available to zero (position.maker === makerLimit).

Example:

BalancedVault balances maker positions between products short ETH and long ETH 50/50.

Alice wants to deposit 10000 DSU into the vault, but Bob seeing this front runs the transaction with creating a maker position topping up the long market to the makerLimit.

After rebalancing, Alice has an exposure of 5000 DSU to short ETH, and can lose money as is not delta neutral as expected.

Impact

A balanced vault can be easily imbalanced during deposits, and users are unexpectedly exposed to one leg of a market.

Code Snippet

Tool used

Manual Review

Recommendation

Maybe one can emulate the same mechanism as during withdrawal: https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-vaults/contracts/balanced/BalancedVault.sol#L672-L674

Where the deposit is capped by makerLimit of each product in every market

[SergeKireev]

Escalate for 10 USDC

As stated in the documentation: https://docs.perennial.finance/lps-makers/vaults

And as seen in the implementation: https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-vaults/contracts/balanced/BalancedVault.sol#L489-L490

Vaults attempt to implement delta-neutral (create same amount of exposure to long and short of assets) liquidity providing strategies. What is demonstrated in this report, is that any external actor can push the vault exposure in any direction by sandwiching deposit transactions.

As a user of a vault, the risk profile is completely changed, because instead of having a delta-neutral exposure, they can end up exposed 100% to short ETH for example, which is worse than funds being idle in the vault waiting to be allocated.

The solution suggested in the report is to, instead of allocating as much as possible in any given market, to allocate as much as possible accordingly to all weights of the vault (note that this is implemented correctly for withdrawals). And most importantly to check it is possible to maintain delta-neutral allocation

Additional note: I forgot to mention in my initial report that Bob can trivially cancel his maker position opening (by closing same amount) after Alice tx, and not actually enter the market.

[sherlock-admin]

Escalate for 10 USDC

As stated in the documentation: https://docs.perennial.finance/lps-makers/vaults

And as seen in the implementation: https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-vaults/contracts/balanced/BalancedVault.sol#L489-L490

Vaults attempt to implement delta-neutral (create same amount of exposure to long and short of assets) liquidity providing strategies. What is demonstrated in this report, is that any external actor can push the vault exposure in any direction by sandwiching deposit transactions.

As a user of a vault, the risk profile is completely changed, because instead of having a delta-neutral exposure, they can end up exposed 100% to short ETH for example, which is worse than funds being idle in the vault waiting to be allocated.

The solution suggested in the report is to, instead of allocating as much as possible in any given market, to allocate as much as possible accordingly to all weights of the vault (note that this is implemented correctly for withdrawals). And most importantly to check it is possible to maintain delta-neutral allocation

Additional note: I forgot to mention in my initial report that Bob can trivially cancel his maker position opening (by closing same amount) after Alice tx, and not actually enter the market.

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]

The vaults don't aim to be delta-neutral, but as the linked doc says, they aim for delta exposure.

However the escalation raises a good point: Bob can trivially cancel his maker position opening (by closing same amount) after Alice tx, and not actually enter the market. So actually Bob can "direct" the Vault's investing for his advantage, eg. by making the vault not enlarge a make position in a product where Bob is making and earning a large funding fee. And this will also make Vault users not invest as per the real state of the market. @arjun-io , please have a look at this issue.

[edit: actually, Bob would also have to pay open and close position fees, which perhaps dampens the efficacy of this strategy.]

[arjun-io]

The vault would rebalance after Bob closes his position - in order to successfully change the position of the vault in their favor they'd need to allocate capital and maintain that position for however long they'd like to alter the vault's position.

[KenzoAgada]

Hmm yes, agreed. The next rebalance would put things to normal. So doesn't seem like a med/high issue.

[SergeKireev]

[edit: actually, Bob would also have to pay open and close position fees, which perhaps dampens the efficacy of this strategy.]

As per current market parameters there are no open/close fees on maker positions.

The vault would rebalance after Bob closes his position - in order to successfully change the position of the vault in their favor they'd need to allocate capital and maintain that position for however long they'd like to alter the vault's position.

You are right I somehow overlooked this: that any action following this operation would put the vault back in the right state by rebalancing. This made me think a bit, and there is a very simple strategy which can allow Bob to make this Dos in an even more efficient way: Bob requests to open the large maker position right after an oracle update (right after settlement), and closes it by front running next oracle update for a given product (which means the position is never actually updated). This way Bob efficiently keeps the vault only invested on one product at all times and independently of other participant actions.

[arjun-io]

This is very difficult to frontrun on a sequencer based chain (where vaults are deployed). This is basically a sandwich attack which could be used to DoS any number of actions

[SergeKireev]

This is very difficult to frontrun on a sequencer based chain (where vaults are deployed). This is basically a sandwich attack which could be used to DoS any number of actions

Agreed that in the current state of affairs Vaults are only deployed on Arbitrum (sequencer based), however I did not find anywhere in the docs that Vaults are to be restricted to Arbitrum.

In the contest page we can see Perennial is deployed on Arbitrum and ETH mainnet, so one can only suppose that the scope applies to both networks similarly: https://audits.sherlock.xyz/contests/79

[jacksanford1]

Yes, the contest should assume ETH mainnet is also a viable chain. @SergeKireev

However, I'm inclined to call this issue invalid because the original premise is no longer true:

any action following this operation would put the vault back in the right state by rebalancing

[SergeKireev]

@jacksanford1 The initial report described how the vault could be intentionally put into an invalid state.

Now regarding the rebutal:

any action following this operation would put the vault back in the right state by rebalancing

Even though true, there is no guarantee on the frequency of actions operated on the vault. The vault may stay in the invalid state for an arbitrary amount of time

Additionally my latest reply showed a way for a malicious user to keep it in the invalid state indefinitely on ETH mainnet.

[jacksanford1]

Appreciate the comments @SergeKireev. I still don't feel like I have a great handle on this issue.

So only a malicious person would do this. What would they gain from it?

And if Alice is the victim, could she also fight back with a second/deposit action in order to strategically defend against this?

Maybe @KenzoAgada @roguereddwarf can help me understand more about the other side of the argument (why this issue should stay invalid).

[KenzoAgada]

@SergeKireev earlier I wrote that Bob could use this to avoid Alice opening a position in a market where Bob is earning funding fee. Other than that, is there another self-gain reason for Bob to do this?

[SergeKireev]

@KenzoAgada @jacksanford1

And if Alice is the victim, could also fight back with a second/deposit action in order to strategically defend against this?

No, in the 'forced' scenario described in my last comment, Alice can not 'fight back'

Not sure if there can be a profit motive for a malicious user to do this. IMO It can be a grieving resulting in the loss of funds for a vault (due to it taking directional risk in the market where it should not as per lines: https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-vaults/contracts/balanced/BalancedVault.sol#L489-L490).

If you deem this as a Low impact (due to the delta neutralness not being a hard requirement), we can classify this as Low. Otherwise it can be a medium I think.

[KenzoAgada]

Thanks @SergeKireev.

Due to the fact that Bob has no real reason to do this, and the relatively complex operation/cost (mentioned in the comments above), I would consider this a low.

[jacksanford1]

If you deem this as a Low impact (due to the delta neutralness not being a hard requirement), we can classify this as Low.

Thanks for the comment @SergeKireev, it helped to clear things up for me. I would agree with you and deem that scenario as low impact. So would consider this issue to be Low (invalid).

[jacksanford1]

Result: Invalid Unique Invalid based on conversation in previous few comments.

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• SergeKireev: rejected