sherlock-audit / 2024-04-titles-judging

1 stars 1 forks source link

Bigsam - Potential Precision Loss in Protocol Share Value #384

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

Bigsam

medium

Potential Precision Loss in Protocol Share Value

Summary

The contract is susceptible to potential precision loss in the protocol share value calculation, which could result in inaccuracies and financial losses. This vulnerability arises due to the disparity in the range of possible values between revshareBps and attributionShares, leading to unexpected behavior when calculating the protocol share value.

Vulnerability Detail

The vulnerability occurs in the calculation of attributionRevShare, where revshareBps is multiplied by 100 and divided by attributionShares. Since revshareBps is limited to a range between 95000 and 250, and attributionShares is casted to a uint32 with a maximum value of 4,294,967,295, there is a high likelihood of precision loss when attributionShares exceeds the available range of revshareBps * 100. This can lead to incorrect calculation results, where even small fractional values are rounded down to zero, resulting in a protocol share value of zero.

Impact

The precision loss in the protocol share value calculation can have a significant impact on the distribution of rewards within the protocol. Incorrect calculation results may lead to undervaluation or overvaluation of protocol shares, potentially causing financial losses for stakeholders and impacting the overall stability and fairness of the protocol.

Code Snippet

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/fees/FeeManager.sol#L482-L483

https://github.com/sherlock-audit/2024-04-titles/blob/main/wallflower-contract-v2/src/fees/FeeManager.sol#L469-L499

Tool used

Manual Review

Recommendation

To mitigate the potential precision loss in the protocol share value calculation and ensure accurate distribution of rewards, the following mitigation is recommended:

Introduce a Multiplier to Ensure Range Compatibility

Implement a multiplier that adjusts the calculation of attributionRevShare based on the ratio between revshareBps and attributionShares. This multiplier ensures that the value of revshareBps * 100 is always greater than attributionShares, preventing precision loss and inaccuracies in the calculation.


uint32 attributionRevShare = revshareBps*100 * multiplier / uint32(attributionShares);
uint32 creatorShare = 1e6 - ((attributionRevShare * attributionShares)/multiplier);

Duplicate of #201

BiasedMerc commented 1 month ago

No POC or even walkthrough of numbers that would lead to relevant precision loss? Seems this should be invalidated as there has not been shown an actual impact from any precision loss, just a generic observation that some precision loss can happen, but with no actual POC showing a relevant scenario and impact.