Submission Details
Severity:
[H-2] _withdraw::SablierFlow breaks the invariant ∑ stream balances + protocol revenue = aggregate balance
Description
function _withdraw(
uint256 streamId,
address to,
uint128 amount
)
internal
returns (uint128 withdrawnAmount, uint128 protocolFeeAmount)
{
// Check: the withdraw amount is not zero.
if (amount == 0) {
revert Errors.SablierFlow_WithdrawAmountZero(streamId);
}
// Check: the withdrawal address is not zero.
if (to == address(0)) {
revert Errors.SablierFlow_WithdrawToZeroAddress(streamId);
}
// Check: `msg.sender` is neither the stream's recipient nor an approved third party, the withdrawal address
// must be the recipient.
if (to != _ownerOf(streamId) && !_isCallerStreamRecipientOrApproved(streamId)) {
revert Errors.SablierFlow_WithdrawalAddressNotRecipient({ streamId: streamId, caller: msg.sender, to: to });
}
uint8 tokenDecimals = _streams[streamId].tokenDecimals;
// Calculate the total debt.
uint256 totalDebtScaled = _ongoingDebtScaledOf(streamId) + _streams[streamId].snapshotDebtScaled;
uint256 totalDebt = Helpers.descaleAmount(totalDebtScaled, tokenDecimals);
// Calculate the withdrawable amount.
uint128 balance = _streams[streamId].balance;
uint128 withdrawableAmount;
if (balance < totalDebt) {
// If the stream balance is less than the total debt, the withdrawable amount is the balance.
withdrawableAmount = balance;
} else {
// Otherwise, the withdrawable amount is the total debt.
withdrawableAmount = totalDebt.toUint128();
}
// Check: the withdraw amount is not greater than the withdrawable amount.
if (amount > withdrawableAmount) {
revert Errors.SablierFlow_Overdraw(streamId, amount, withdrawableAmount);
}
// Calculate the amount scaled.
uint256 amountScaled = Helpers.scaleAmount(amount, tokenDecimals);
// Safe to use unchecked, `amount` cannot be greater than the balance or total debt at this point.
unchecked {
// If the amount is less than the snapshot debt, reduce it from the snapshot debt and leave the snapshot
// time unchanged.
if (amountScaled <= _streams[streamId].snapshotDebtScaled) {
_streams[streamId].snapshotDebtScaled -= amountScaled;
}
// Else reduce the amount from the ongoing debt by setting snapshot time to `block.timestamp` and set the
// snapshot debt to the remaining total debt.
else {
_streams[streamId].snapshotDebtScaled = totalDebtScaled - amountScaled;
// Effect: update the stream time.
_streams[streamId].snapshotTime = uint40(block.timestamp);
}
// Effect: update the stream balance.
_streams[streamId].balance -= amount;
}
// 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.
(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.
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);
}
In the Technical Doc, we have a variant
∑ stream balances + protocol revenue = aggregate balance.........................(i)
In the _withdrawfunction
aggregateBalanceand _streams[streamId].balancewere deducted by the amount to be withdrawn respectively, if the protocolFee > 0, protocolRevenue value will be increased by a proctocolFee, thereby causing the left hand side of equation (i) > than the right hand side
Impact: **_withdraw**could break one of the protocol invariants
Recommended mitigation
If the protocolFee > 0, the protocolFee should first be decucted from the amount to be withdrawn in a new variable say amountAfterProtocolFee, then
function _withdraw(
uint256 streamId,
address to,
uint128 amount
)
internal
returns (uint128 withdrawnAmount, uint128 protocolFeeAmount)
{
// Check: the withdraw amount is not zero.
if (amount == 0) {
revert Errors.SablierFlow_WithdrawAmountZero(streamId);
}
// Check: the withdrawal address is not zero.
if (to == address(0)) {
revert Errors.SablierFlow_WithdrawToZeroAddress(streamId);
}
// Check: `msg.sender` is neither the stream's recipient nor an approved third party, the withdrawal address
// must be the recipient.
if (to != _ownerOf(streamId) && !_isCallerStreamRecipientOrApproved(streamId)) {
revert Errors.SablierFlow_WithdrawalAddressNotRecipient({ streamId: streamId, caller: msg.sender, to: to });
}
uint8 tokenDecimals = _streams[streamId].tokenDecimals;
// Calculate the total debt.
uint256 totalDebtScaled = _ongoingDebtScaledOf(streamId) + _streams[streamId].snapshotDebtScaled;
uint256 totalDebt = Helpers.descaleAmount(totalDebtScaled, tokenDecimals);
// Calculate the withdrawable amount.
uint128 balance = _streams[streamId].balance;
uint128 withdrawableAmount;
// Load the variables in memory.
IERC20 token = _streams[streamId].token;
UD60x18 protocolFee = protocolFee[token];
if (balance < totalDebt) {
// If the stream balance is less than the total debt, the withdrawable amount is the balance.
withdrawableAmount = balance;
} else {
// Otherwise, the withdrawable amount is the total debt.
withdrawableAmount = totalDebt.toUint128();
}
if (protocolFee > ZERO) {
// Calculate the protocol fee amount and the net withdraw amount.
(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;
}
}
uint128 amountAfterProtocolFee = amount - protocolFee;
// Check: the withdraw amount is not greater than the withdrawable amount.
if (amountAfterProtocolFee > withdrawableAmount) {
revert Errors.SablierFlow_Overdraw(streamId, amountAfterProtocolFee , withdrawableAmount);
}
// Calculate the amount scaled.
uint256 amountScaled = Helpers.scaleAmount(amountAfterProtocolFee , tokenDecimals);
// Safe to use unchecked, `amount` cannot be greater than the balance or total debt at this point.
unchecked {
// If the amount is less than the snapshot debt, reduce it from the snapshot debt and leave the snapshot
// time unchanged.
if (amountScaled <= _streams[streamId].snapshotDebtScaled) {
_streams[streamId].snapshotDebtScaled -= amountScaled;
}
// Else reduce the amount from the ongoing debt by setting snapshot time to `block.timestamp` and set the
// snapshot debt to the remaining total debt.
else {
_streams[streamId].snapshotDebtScaled = totalDebtScaled - amountScaled;
// Effect: update the stream time.
_streams[streamId].snapshotTime = uint40(block.timestamp);
}
// Effect: update the stream balance.
_streams[streamId].balance -= amountAfterProtocolFee ;
}
unchecked {
// Effect: update the aggregate balance.
aggregateBalance[token] -= amountAfterProtocolFee ;
}
// Interaction: perform the ERC-20 transfer.
token.safeTransfer({ to: to, value:amountAfterProtocolFee });
// 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 (amountAfterProtocolFee , protocolFeeAmount);
}
Updates
Prize pool breakdown
Total prize
20,000 USDC
nSLOC
69929 USDC / LOC
19,000 USDC
1,000 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.