franfran20 - Member Units Can Be Lost Due To Overwriting Nature Of UpdateUserUnits

[sherlock-admin2]

franfran20

High

Member Units Can Be Lost Due To Overwriting Nature Of UpdateUserUnits

Summary

When a locker receives a signature from the stackSigner to claim units allocated to them, the previous units are overwritten. On stack it is not unusual for points(units) to be awarded based on events that occur on chain, so if a locker claims one of their units and hasn't claimed another yet for the same program. When the locker owner decides to claim their units via the FluidLocker:claim() function.

The nature of the function tends to overwrite the previous units with the old ones rather than compound the units for the same program the way its done for stakers stake amount in the tax distribution pool.
In a situation where the second units claimed by the locker is less than the initial, this basically leads to an invalid drop off in share of the distribution pool for that program.

https://github.com/sherlock-audit/2024-11-superfluid-locking-contract/blob/main/fluid/packages/contracts/src/FluidLocker.sol#L155-L171

Root Cause

The root cause of this issue happens in the internal _poolUpdate function in the EPProgramManager, when locker owner claims units in the their locker contract it calls the FluidProgramManager which has the EPProgramManager which contains the function to handle the claim, updateUserUnits, which contains the _poolUpdate function that contains the super token logic to update the lockers units share in the program distribution pool.

https://github.com/sherlock-audit/2024-11-superfluid-locking-contract/blob/main/fluid/packages/contracts/src/EPProgramManager.sol#L146

The nature of the function in the super token is to overwrite the units of the member with the new specified units rather than compound them, why you see the design choice when handling stakers amount to the tax distribution pool here:

Where the total staked balance was passed rather than the newly staked amount
https://github.com/sherlock-audit/2024-11-superfluid-locking-contract/blob/main/fluid/packages/contracts/src/FluidLocker.sol#L230-L250

Internal pre-conditions

1. The locker has two different set of units received for the program pool (say 100 units and 50 units).
2. The locker has claimed the 100 units but is yet to claim the 50.

External pre-conditions

None

Attack Path

The locker claims the other set of units available to them (50 units)

This overwrites the initial units owned by the locker with the new units claimed i.e instead of the user having 150 units, they have 50 units.

Impact

The locker loses their rightfully claimed units share in the program distribution pool and in return the rate at which tokens flow to them is reduced as well because their units were overwritten instead of compounded.

PoC

No response

Mitigation

The same way units were compounded (accounted for) with lockers staking in the tax distribution pool, units should be accounted for and compounded in the program distribution pool when new units are claimed.