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

Incorrect Transfer Amount in Function Due to Protocol Fee Deduction

aycozynfada

Summary

The withdraw function in sablierFlow.sol incorrectly transfers the total amount to a user instead of transferring the net amount after deducting the protocol fee. This results in the protocol fee not being properly accounted for in the transfer.

Vulnerability Details

The function calculates the protocol fee and updates the protocol revenue, but it transfers the total amount to the recipient instead of the net amount after deducting the protocol fee.

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;//subtracts total amount from aggregate
}
​
// Interaction: perform the ERC-20 transfer.
token.safeTransfer({ to: to, value: amount });// transfers total amount to recipient

Impact

The recipient receives the total amount instead of the net amount after deducting the protocol fee, leading to financial discrepancies in protocol state. Therefore protocol has less tokens than recorded

Tools Used

manual review

Recommendations

Modify the function to transfer the net amount to the recipient after deducting the protocol fee.
solidity

if (protocolFee > ZERO) {
// Calculate the protocol fee amount and the net withdraw amount.
++ (protocolFeeAmount, Netamount) = 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: Netamount });// transfers NetAmount to recipient

}

unchecked {
// Effect: update the aggregate balance.
aggregateBalance[token] -= (amount + protocolFeeAmount); // subtract total amount including fee from aggregate
}

// Interaction: perform the ERC-20 transfer.
token.safeTransfer({ to: to, value: amount - protocolFeeAmount }); // transfers net amount to recipient

Updates

Lead Judging Commences

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

Support

FAQs

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