Flow
Sablier
FoundryDeFi
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

"Improper Input Validation: Missing Zero Check for `totalAmount` in ProtocolFee and net WIthdraw amount Calculation

eagles

Summary

The SablierFlow::depositViaBroker function manages fund deposits through a broker, while the internal SablierFlow::_depositViaBroker function verifies the broker and calculates the deposit amount. This internal function calls the Helpers::checkAndCalculateBrokerFee function to determine the broker fee and deposit amount based on the totalAmount.

However, the Helpers::checkAndCalculateBrokerFee function then relies on Helpers::calculateAmountsFromFee to compute the fee and net amount by subtracting the fee from the `totalAmount, according to a specified fee percentage. None of these functions validate that the totalAmount is not zero , even though the fee and deposit amounts are calculated from it.

Additionally, Helpers::calculateAmountsFromFee is also invoked in the SablierFlow::_withdraw internal function, which manages withdrawals and calculates the protocol fee and net withdrawal amount. Without validating that the totalAmount is not zero, the calculations of both the protocol fees and net withdrawal amounts may be inaccurate.

function calculateAmountsFromFee(
uint128 totalAmount,
UD60x18 fee
)
internal
pure
returns (uint128 feeAmount, uint128 netAmount)
{
//@audit no zero total amount checks
// Calculate the fee amount based on the fee percentage.
feeAmount = ud(totalAmount).mul(fee).intoUint128(); // Todo
​
// Calculate the net amount after subtracting the fee from the total amount.
netAmount = totalAmount - feeAmount;
}

Vulnerability Details

The Helpers::calculateAmountsFromFee function is utilized within the Helpers::checkAndCalculateBrokerFee and SablierFlow::_withdraw function, but neither of these functions currently includes a validation to confirm that the totalAmount is not zero. This lack of validation can result in calculation inconsistencies and unpredictable behavior. Specifically, when Helpers::calculateAmountsFromFee is called in Helpers::checkAndCalculateBrokerFee and SablierFlow::_withdrawto calculate the fees and net amounts, a zero totalAmount can lead to errors during broker deposits or when calculating protocol fees and net withdrawal amounts.

​
​
function _depositViaBroker(uint256 streamId, uint128 totalAmount, Broker memory broker) internal {
// Check: verify the `broker` and calculate the amounts.
(uint128 brokerFeeAmount, uint128 depositAmount) =
1 Helpers.checkAndCalculateBrokerFee(totalAmount, broker, MAX_FEE);
​
// Checks, Effects, and Interactions: deposit on stream.
_deposit(streamId, depositAmount);
​
// Interaction: transfer the broker's amount.
_streams[streamId].token.safeTransferFrom({ from: msg.sender, to: broker.account, value: brokerFeeAmount });
}
​
​
​
​
function checkAndCalculateBrokerFee(
uint128 totalAmount,
Broker memory broker,
UD60x18 maxFee
)
internal
pure
returns (uint128 brokerFeeAmount, uint128 depositAmount)
{
// Check: the broker's fee is not greater than `MAX_FEE`.
if (broker.fee.gt(maxFee)) {
revert Errors.SablierFlow_BrokerFeeTooHigh(broker.fee, maxFee);
}
​
// Check: the broker recipient is not the zero address.
if (broker.account == address(0)) {
revert Errors.SablierFlow_BrokerAddressZero();
}
​
// Calculate the broker fee amount that is going to be transferred to the `broker.account`.
2 (brokerFeeAmount, depositAmount) = calculateAmountsFromFee(totalAmount, broker.fee);
}
​

SablierFlow::_withdrawcode block below

// Load the variables in memory.
IERC20 token = _streams[streamId].token;
UD60x18 protocolFee = protocolFee[token];
​
if (protocolFee > ZERO) {
// Calculate the protocol fee amount and the net withdraw amount.
3 (protocolFeeAmount, amount) = Helpers.calculateAmountsFromFee({ totalAmount: amount, fee: protocolFee });
​
// Safe to use unchecked because addition cannot overflow.
unchecked {
// Effect: update the protocol revenue.
protocolRevenue[token] += protocolFeeAmount;
}
}
​

Impact

It can lead to an unexpected behavior when the Protocolfee and net withdraw amount are being calculated

Tools Used

Manual Review

Recommendations

Include a check that will ensure that the totalAmount is not zero in the Helpers::calculateAmountsFromFeeto ensure totalAmount can not be zero.

Note: The error message can be changed according to the protocol chioce of error message.

function calculateAmountsFromFee(
uint128 totalAmount,
UD60x18 fee
)
internal
pure
returns (uint128 feeAmount, uint128 netAmount)
{
// Check if the total amount is zero and revert with an appropriate error message.
++ if (totalAmount == 0) {
++ revert Errors.SablierFlow_TotalAmountCannotBeZero();
}
​
// Calculate the fee amount based on the fee percentage.
feeAmount = ud(totalAmount).mul(fee).intoUint128();
​
// Calculate the net amount after subtracting the fee from the total amount.
netAmount = totalAmount - feeAmount;
}
​
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!