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

The Protocol and Broker are prohibited from charging a maximum fee of 1e18 (100%). Consequently, they incur a 10% loss on any transaction amount.

theirrationalone

Summary

According to the NatSpec documentation, the maximum fee that a broker and the protocol can charge, essential for earning
brokerage and enhancing revenue, is set at 1e18, a constant value defined by the contract's developer. However, there is
a mistake that the developer(s) may inadvertently set the SablierFlowBase::MAX_FEE to 0.1e18 (equivalent to 1e17)
instead of the intended 1e18.

Referencing SablierFlowBase::MAX_FEE:
Repo Link:

/// @inheritdoc ISablierFlowBase
UD60x18 public constant override MAX_FEE = UD60x18.wrap(0.1e18);
--------------------------------------------------------------^

According to the NatSpec documentation:
Repo Link

  • The documentation states:
    @notice Retrieves the maximum fee that can be charged by the broker and the protocol, denoted as a fixed-point percentage where 1e18 represents 100%.

Vulnerability Details

To demonstrate this issue, please follow the steps outlined below:

  1. Add the following test to one of your test files:

SablierFlow sabFlow;
​
address theAdmin = makeAddr("THE ADMIN");
address theSender = makeAddr("THE SENDER");
address theRecipient = makeAddr("THE RECIPIENT");
UD21x18 rpsNewDef = UD21x18.wrap(2e18);
IERC20 newToken;
​
function setUp() public {
ERC20Mock newTk = new ERC20Mock("My Token", "MTKN", 18);
newToken = IERC20(address(newTk));
FlowNFTDescriptor flowNftDes = new FlowNFTDescriptor();
sabFlow = new SablierFlow(theAdmin, flowNftDes);
deal(address(newToken), theSender, 1000e18);
}
​
function testAdminCannotSetMaxFee() public {
uint256 maxFee = 1e18;
uint256 allowedMaxFee = 0.1e18;
UD60x18 allowedMaxFeeConv = UD60x18.wrap(allowedMaxFee);
UD60x18 pFee = UD60x18.wrap(maxFee);
vm.startPrank(theAdmin);
vm.expectRevert(
abi.encodeWithSelector(Errors.SablierFlowBase_ProtocolFeeTooHigh.selector, maxFee, allowedMaxFeeConv)
);
sabFlow.setProtocolFee(newToken, pFee);
vm.stopPrank();
}

  1. Open your terminal and execute the following command:

forge test --mt testAdminCannotSetMaxFee

  1. Review the logs:

[⠊] Compiling...
[⠊] Compiling 1 files with Solc 0.8.26
[⠒] Solc 0.8.26 finished in 3.83s
Compiler run successful!
​
Ran 1 test for tests/integration/concrete/depletion-time-of/depletionTimeOf.t.sol:SablierTesting
[PASS] testAdminCannotSetMaxFee() (gas: 16212)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.06ms (114.50µs CPU time)
​
Ran 1 test suite in 7.87ms (2.06ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

As evidenced, the admin cannot set the maximum fee.

Impact

  • The Protocol and Broker will incur a 10% loss on any total amount they wish to charge.

  • Since this value is constant, it cannot be changed, but it restricts the admin from setting a fee exceeding the
    maximum allowed.

  • This issue persists even after deployment, as the admin of the protocol cannot alter this rule due to the nature of
    MAX_FEE.

Tools Used

  • Manual Review

Recommendations

It is essential to update the SablierFlowBase::MAX_FEE before deploying the contract. A suggested solution is as
follows:

SablierFlowBase.sol

...
...
​
/// @inheritdoc ISablierFlowBase
- UD60x18 public constant override MAX_FEE = UD60x18.wrap(0.1e18);
+ UD60x18 public constant override MAX_FEE = UD60x18.wrap(1e18);
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.