Summary

An important component of this protocol is that users who have created short positions be able to earn all the staking yield (sub fees) from the underlying LST collateral. However, as currently implemented, there is nothing preventing a malicious user from creating a short record during the last second and earn the same fees as a user who has had their short position outstanding for a much longer period of time. This is unfair, as users who have been around longer should effectively get a greater amount of LST yield, than users who have just started shorting.

Vulnerability Details

When a short order is matched with bid order, matchIncomingShort is called, which has the following snippet of code:

Note that when creating the short record they simply take the cached Vault.zethYieldRate which can be significantly less than the actual yield rate at this point in time. This is because Vault.zethYieldRate is only updated when a user manually calls the updateYield function of the YieldFacet contract.

Recall how the ETH yield for a user is calculated in the _distributeYield function of the YieldFacet contract:

Effectively the zethYieldRate is used the same way as reward debt is for MasterChef, meaning a user can perform the following attack to unfairly get excessive amounts of LST collateral yield:

1. During the same block as there's an increase in the yield from the LST collateral (e.g. rebasing block for stETH), backrun the rebase with a call to create a huge short record (potentially matched against user's own bid)

2. Call LibVault:updateYield(..), which will increases Vault.zethYieldRate

3. Call YieldFacet:distributeYield(..), which will increase the user's ethEscrowed

4. Exit the short position

Impact

A malicious user is able to create a short record immediately prior to a call to updateYield and collect an unfair amount of LST collateral yield, at the expense of other users who have had open short positions for significantly longer.

Tools Used

Manual review

Recommendations

The updateYield function should arguably be called prior to every attempt to create a new short record, in order to ensure that Vault.zethYieldRate is up to date.