Summary

https://github.com/Cyfrin/2025-01-zaros-part-2/blob/39e33b2f6b3890573bb1affc41a7e520277ceb2c/src/market-making/leaves/Distribution.sol#L28

The contract relies on the distributeValue function to allocate a value across shares, but it may not properly handle edge cases such as zero shares or extremely small or large values. This can lead to incorrect value distribution, where actors either receive incorrect amounts or the contract fails to distribute values as intended.

Vulnerability Details

distributeValue(Data storage self, SD59x18 value):

The function attempts to distribute a value among all actors based on their shares. However, if the totalShares is zero (i.e., no actors have shares), it will fail by reverting with Errors.EmptyDistribution(). In some cases, such as when dealing with very small values or precision issues, the function may not distribute values correctly or may cause unexpected behavior.

setActorShares(Data storage self, bytes32 actorId, UD60x18 newActorShares):

The shares assigned to an actor are updated, but if there is an issue with the way the shares are being calculated, actors may receive more or less than intended. This can result in uneven value distribution.

_updateLastValuePerShare(Data storage self, Actor storage actor, UD60x18 newActorShares):

This function updates the actor's last value per share, but if there are issues with the calculation or actors' shares are set incorrectly, the stored values may be inconsistent with the actual distribution.

Root Cause

Division by Zero: In the distributeValue function, the total number of shares is converted to UD60x18, and if there are no shares (totalShares == 0), division by zero occurs. Although this is caught by the Errors.EmptyDistribution() revert, it is still a potential issue if not handled properly in the logic.

Rounding Issues: When dividing values with high precision (such as SD59x18 and UD60x18 types), rounding errors can cause incorrect calculations in certain cases, especially when values are very small or very large. These rounding errors could lead to inaccurate distributions of values.

Actor Share Updates: The logic in setActorShares and _updateLastValuePerShare might not properly account for the overall distribution, especially when shares are updated dynamically. If the shares are not properly updated or calculated, the actor's final distribution could be incorrect.

Impact

Incorrect Distribution: Actors may receive incorrect or skewed values, resulting in financial discrepancies.

Loss of Funds: Misallocation of values can cause actors to either receive more than their fair share or be undercompensated.

Unintended Behavior: If the value distribution doesn't work as intended, the contract may not fulfill its purpose of allocating values fairly among actors.

Tools Used

Manual Review

Recommendations

Ensure Safe Division: Ensure that the division operation in distributeValue checks for a non-zero total share before performing the division to avoid division by zero errors. You may want to return or revert with a specific error if totalShares is zero, signaling an invalid distribution state.

Handle Rounding Issues: When working with high-precision types like SD59x18 and UD60x18, ensure proper rounding is applied to avoid discrepancies in small or large value calculations. Consider using rounding functions or scaling values to an acceptable precision before performing operations.

Update Actor Shares Correctly: When updating actor shares, make sure the share calculations are consistent and reflect any changes in the overall shares. This ensures the correct distribution of values when shares are updated or actors are added or removed.

Perform Unit and Integration Testing: Test the contract with a variety of edge cases, such as having zero total shares, zero actor shares, and very small or large values, to ensure that value distribution behaves as expected in all scenarios.

Use Revert Errors with Specific Messaging: Instead of using a general revert in distributeValue when the total shares are zero, provide more specific revert messages or implement fallback mechanisms to handle edge cases more gracefully.