Summary

In VaultRouterBranch- the stakefunction allows LP providers to stake the vault shares that they received from depositing collateral specific to the vaultId.

The vault periodically, positively updates its valuePerShare amount when value is generated and distributed to the vault - this value is used to determine LP providers value earned per share that they staked.

The Distributioncontract is responsible for accounting for those values along with the specific LP providers share amount and valuePerShare.

We will go through a 2 step process in the scenario when an LP Provider stakes into the same vault twice, in which they will lose all value that their inital staked amount earned them.

Vulnerability Details

• The LP provider initially stakes 20 shares to a vault by calling stake.

• recalculateVaultsCreditCapacitydistributes value to the vault and updates the valuePerShareparamater in Distributioncontract.

  • lets assume the valuePerShareis updated to 2.

• In the Distributioncontract this will be the state of the LP provider:

  • shares = 20

  • actor.lastValuePerShare= 2

  • Global valuePerShare =2

SECOND STAKE

For this second stake, lets assume these values for the stake (for simplicity):

New shares to stake = 10

The valuePerSharegets value distributed to it and increases by 2. (from 2 to 4).

• The LP provider calls stakeagain with the same VaultID.

• recalculateVaultsCreditCapacity - distributes value to the Distributioncontract and the global variable valuePerShareis increased by 2. (from 2 -> 4)

The follwoign will be the flow from staketo Distributor- showing how stakeupdates the values in Distributor- which causes the LP Provider to lose funds

1. stakecalls accumulateActorin the Distributecontract

** Distribution contract flow at that point:**

As a result:

The value change of 40was calculated -> representing the amount of value that the LP provider had accumulated from staking 20 shares -> and staking for a duration that saw the vault generate value (which increased the valuePerSharefrom 2 -> 4)

The lastValuePerSharefor the LP provider is now updated to 4.

Next, stakecalls setActorShares-> passing 30 for updatedActorShares ** (20 original shares + 10 new shares) **

****

The following is the flow and result of the logic of setActorSharesin the Distributioncontract

1. valueChange = 0

   • (the LP providers lastValuePerSharewas already updated to the current global valuePerShareearlier when stakecalled accumulateActor.-> so there is no value change)

2. actor.shares = 30

   • original 20 shares staked + 10 new shares staked

3. The LP providers final state will be:

   • 30 total shares

   • lastValuePerShareis the current global value, meaning there isnt any value to distribute to them per share

   • valueChangeis 0.

Impact

When stakecalled accumulateActor -> Distributecalculated the correct amount of value that the LP provider earned by staking for a duration that saw the vault generate value.

But that valueChangeamount is never stored anywhere, and there is nothing done with it.

So accumulateActordoes not accumulate value the LP provider has earned, it just calculates it. And then subsequently overrides it to 0.

The LP Provider loses the amount of value that their staked shares generate and then have to start over from 0 when they stake again.

accumulateActorin the stakefunction states :

Which suggests that the valueChange amount that is calculated should be stored for the LP provider to be able to redeem or different functionality -> but one that allows the LP provider to realize the value they earned.

valueChangeis returned by each function seen here in Distribution

But as shown above, the function call to accumulateActordoes not handle the return value -> it gets lost and unused.

Tools Used

Manual Review

Recommendations

1. Because the amount of fee's earned will be overridden and will be 0 after the new stake -> apply the same check used in unstake-> which ensures the staker doesnt have any claimable rewards and only allows the action if they have no rewards to claim:

This adds a restriction on LP providers to only allow a subsequent stake amount only if they claim all of their rewards accumulated at that point.

But it is definitely better than staking more and losing all of the rewards from your previous staked amount and duration staked.