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

Incorrect Aggregate Balance Update in Function Due to Protocol Fee Deduction

aycozynfada

Summary

The withdraw function in sablierFlow.sol incorrectly subtracts the total amount from the aggregate token balance instead of subtracting just the net amount after deducting the protocol fee. This results in an inaccurate aggregate balance. This vulnerability is feasible because the collectProtocolRevenue function in the sablierflowbase.sol also goes ahead to subtract the fee amount from aggregate token balance during its own withdrawal leading to double subtraction from aggregate token balance

Here is the section of the withdraw function from sablierFlow.sol.

the function subtracts Total amount from aggregateTokenBalance

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

Here is the collectProtocolRevenue() in sablierFlowBase.sol which goes on to subtract the protocol fee from aggregate token balance again

function collectProtocolRevenue(IERC20 token, address to) external override onlyAdmin {
uint128 revenue = protocolRevenue[token];
​
// Check: there is protocol revenue to collect.
if (revenue == 0) {
revert Errors.SablierFlowBase_NoProtocolRevenue(address(token));
}
​
// Effect: reset the protocol revenue.
protocolRevenue[token] = 0;
​
unchecked {
// Effect: update the aggregate balance.
aggregateBalance[token] -= revenue;
}
​
// Interaction: transfer the protocol revenue to the provided address.
token.safeTransfer({ to: to, value: revenue });
​
emit ISablierFlowBase.CollectProtocolRevenue({ admin: msg.sender, token: token, to: to, revenue: revenue });
}

Vulnerability Details

The function subtracts the total amount from the aggregate balance instead of the net amount after deducting the protocol fee. This leads to an incorrect aggregate balance.

Impact

The incorrect balance update can cause financial discrepancies, affecting the accuracy of the contract’s financial records, where protocol can have less token than recorded

Tools Used

manual review

Recommendations

Modify the function to subtract the net amount from the aggregate balance after deducting the protocol fee. Or skip the reduction of aggregateToken balance by protocol fee in collectProtocolRevenue

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

Lead Judging Commences

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

Support

FAQs

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