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

Inconsistent fee rate can be applied to `calculateAmountsFromFee` function

0xcodex

Summary

The calculateAmountsFromFee function in the Helpers library, which calculates a fee and net amount based on an input totalAmount and a fee rate (fee). It was identified that inconsistent or varying fee rates can be applied to the same totalAmount, resulting in distinct outputs for different fee rates. This behaviour, though technically functional, could be problematic when higher or lower fee rates are applied leading to users paying varying fees for the same amounts.

Vulnerability Details

The function calculateAmountsFromFee calculates a fee based on a provided feeRate and a totalAmount. The library does not enforce any restrictions or validation on the feeRate parameter, allowing different fee rates to be applied to the same totalAmount. This inconsistency can be problematic in cases where a fixed or standard fee rate is expected.

https://github.com/Cyfrin/2024-10-sablier/blob/8a2eac7a916080f2022527408b004578b21c51d0/src/libraries/Helpers.sol#L15

Please the test below in your test file

import { Test, console } from "node_modules/forge-std/src/Test.sol";
import { Helpers } from "src/libraries/Helpers.sol";
import { ud, UD60x18 } from "@prb/math/src/UD60x18.sol";
​
contract HelpersExposer {
using Helpers for uint128;
​
function calculateAmountsFromFeeExposed(uint128 totalAmount, UD60x18 fee) public pure returns(uint128 feeAmount, uint128 netAmount)
{
return Helpers.calculateAmountsFromFee(totalAmount, fee);
}
}
​
​
contract CalculateAmountsTest is Test {
HelpersExposer exposer;
​
function setUp() public {
exposer = new HelpersExposer();
}
​
​
function testApplyDifferentFeeRates() public {
uint128 totalAmount = 1000;
​
UD60x18 feeRate1 = UD60x18.wrap(0.05e18); //5%
UD60x18 feeRate2 = UD60x18.wrap(0.10e18); //10%
UD60x18 feeRate3 = UD60x18.wrap(0.40e18); //40%
​
(uint128 feeAmount1, uint128 netAmount1) = exposer.calculateAmountsFromFeeExposed(totalAmount, feeRate1);
(uint128 feeAmount2, uint128 netAmount2) = exposer.calculateAmountsFromFeeExposed(totalAmount, feeRate2);
(uint128 feeAmount3, uint128 netAmount3) = exposer.calculateAmountsFromFeeExposed(totalAmount, feeRate3);
​
// Assert that each fee calculation is distinct for different fee rates
assertTrue(feeAmount1 < feeAmount2 && feeAmount2 < feeAmount3, "Fee amounts should vary with fee rates");
assertTrue(netAmount1 > netAmount2 && netAmount2 > netAmount3, "Net amounts should vary inversely with fee rates");
}
}

Impact

This vulnerability could result in inconsistent user experiences, as users may encounter varying fee calculations on identical amounts, leading to confusion and reduced trust in the protocol. Additionally, it opens the door to potential arbitrage, where users might exploit adjustable fee rates to minimize fees, ultimately creating revenue inconsistencies for the protocol as users manipulate rates to lower their obligations.

Tools Used

Manual Review

Recommendations

It is recommended the following actions be taken:

  1. Implement Fixed Fee Rate: If a consistent fee rate is expected, consider hard-coding or standardizing the feeRate value within the contract rather than passing it as a parameter.

  2. Add Validation Checks: To prevent unintended fee rate values, add checks that enforce acceptable ranges for feeRate. This can reduce the risk of extreme values leading to unexpected feeAmount and netAmount results.

While the calculateAmountsFromFee function works as intended, its flexibility in accepting different feeRate values can lead to inconsistencies that may affect user experience and protocol revenue. Applying the recommended fixes will help ensure consistent fee calculations and a more predictable protocol behavior.

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.