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

wrong return price of aggregateBalance balance

unu5464

Summary

The documentation claims that the function retrieves the sum of balances of all streams. However, the calculation omits the fees,
meaning that fees are not included in the sum of the streams' balances. This discrepancy between the documented behavior and actual
implementation could lead to misunderstanding for developers or incorrect assumptions about the total balance available for streaming.

Vulnerability Details

The function documentation states it returns the sum of all streams' balances, but it does not account for the fees deducted.
Since the fees are subtracted and not considered part of the streams' balances, the final balance sum does not match the expectation
set in the documentation. This discrepancy could lead to incorrect total balance calculations when fees are present,
affecting both auditing processes and potential user balances when interacting with the contract.

link: https://github.com/Cyfrin/2024-10-sablier/blob/main/src/interfaces/ISablierFlowBase.sol#L61-L63

/// @notice Retrieves the sum of balances of all streams.
/// @param token The ERC-20 token for the query.
function aggregateBalance(IERC20 token) external view returns (uint256);
function _withdraw(uint256 streamId, address to, uint128 amount) internal returns(uint128 withdrawnAmount, uint128 protocolFeeAmount) {
//....
IERC20 token = _streams[streamId].token;
UD60x18 protocolFee = protocolFee[token];
​
if (protocolFee > ZERO) {
// Calculate the protocol fee amount and the net withdraw amount.
code (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;
}
}
​
unchecked {
// Effect: update the aggregate balance.
code aggregateBalance[token] -= amount;
}
​
// Interaction: perform the ERC-20 transfer.
token.safeTransfer({ to: to, value: amount });
​
// Protocol Invariant: the difference in total debt should be equal to the difference in the stream balance.
assert(totalDebt - _totalDebtOf(streamId) == balance - _streams[streamId].balance);
​
// Log the withdrawal.
emit ISablierFlow.WithdrawFromFlowStream({streamId: streamId, to: to, token: token, caller: msg.sender, withdrawAmount: amount, protocolFeeAmount: protocolFeeAmount });
return (amount, protocolFeeAmount);
}

Imapct

If the protocol takes fees the aggregateBalance will return wrong information about all streams balances.

Recommendations

Amend the documentation to clarify that the sum of balances does not include fees deducted.

Updates

Lead Judging Commences

inallhonesty Lead Judge
8 months ago
inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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