Sablier
Sablier
DeFiFoundry
53,440 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Rounding down of broker Fees is not in the favor of Protocol

tripathi

Summary

protocol takes broker fees when a sender creates a stream. Broker fees is a one of the revenue source of protocol. As a traditional DeFi rule says rounding should be in the favor of the protocol but in our case it's not in the favor of protocol which allows zero broker fees for tiny amount

Vulnerability Details

File: v2-core/src/libraries/Helpers.sol
​
function checkAndCalculateBrokerFee(
uint128 totalAmount,
UD60x18 brokerFee,
UD60x18 maxBrokerFee
)
internal
pure
returns (Lockup.CreateAmounts memory amounts)
{
// When the total amount is zero, the broker fee is also zero.
if (totalAmount == 0) {
return Lockup.CreateAmounts(0, 0);
}
​
// Check: the broker fee is not greater than `maxBrokerFee`.
if (brokerFee.gt(maxBrokerFee)) {
revert Errors.SablierV2Lockup_BrokerFeeTooHigh(brokerFee, maxBrokerFee);
}
​
// Calculate the broker fee amount.
// The cast to uint128 is safe because the maximum fee is hard coded.
amounts.brokerFee = uint128(ud(totalAmount).mul(brokerFee).intoUint256());
​
// Assert that the total amount is strictly greater than the broker fee amount.
assert(totalAmount > amounts.brokerFee);
​
// Calculate the deposit amount (the amount to stream - net of the broker fee).
amounts.deposit = totalAmount - amounts.brokerFee;
}

amounts.brokerFee = uint128(ud(totalAmount).mul(brokerFee).intoUint256()) here we can see that brokerFee is calculated by rounding down which is not in the favor of protocol.

Users can use this to make brokerFee zero for tiny amount where protocol will incurred loss

Impact

protocol will incur tiny loss in every stream creation

Tools Used

Manual

Recommendations

Instead of rounding down brokerfee, round down deposit amount and then subtract from total amount to get broker Fees.
A good example can be seen below

function checkAndCalculateBrokerFee(
uint128 totalAmount,
UD60x18 brokerFee,
UD60x18 maxBrokerFee
)
internal
pure
returns (Lockup.CreateAmounts memory amounts)
{
// When the total amount is zero, the broker fee is also zero.
if (totalAmount == 0) {
return Lockup.CreateAmounts(0, 0);
}
​
// Check: the broker fee is not greater than `maxBrokerFee`.
if (brokerFee.gt(maxBrokerFee)) {
revert Errors.SablierV2Lockup_BrokerFeeTooHigh(brokerFee, maxBrokerFee);
}
​
// Calculate the broker fee amount.
// The cast to uint128 is safe because the maximum fee is hard coded.
// amounts.brokerFee = uint128(ud(totalAmount).mul(brokerFee).intoUint256());
amounts.deposit= uint128(ud(totalAmount).mul(UD60x18.wrap(1e18)-brokerFee).intoUint256());
// Assert that the total amount is strictly greater than the broker fee amount and deposit fee.
assert(totalAmount > amounts.deposit);
​
// Calculate the broker amount (the amount to stream - deposit amount).
amounts.brokerFee = totalAmount - amounts.deposit;
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.