Flow

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

Immediate Effect of Fee Changes Causing Unanticipated Financial Impacts on Users

Summary

An immediate effect of fee changes in the SablierFlow smart contract presents a vulnerability where sudden adjustments to fees can adversely impact users without prior notice. This issue opens up the possibility for front-running attacks, where malicious actors exploit the lack of delay between fee updates and their effectiveness.

Vulnerability Details

In the SablierFlow contract, administrators have the authority to adjust fee-related parameters instantly. This immediate effect means that any changes to fees take place without a grace period, catching users off-guard. For instance, if a user initiates a transaction assuming a certain fee structure, but the administrator changes the fees right before the transaction is mined, the user may end up paying higher fees than expected.

Example Scenario:

  1. User Interaction:

    • Alice initiates a stream creation, expecting to pay a 1% broker fee.

    • She submits the transaction to the network.

  2. Fee Adjustment:

    • Before Alice's transaction is mined, the administrator increases the broker fee to 5%.

    • This change takes effect immediately.

  3. Result:

    • Alice's transaction is mined with the new 5% fee instead of the expected 1%.

    • Alice pays more than anticipated without any prior notice.

Underlying Issue:

The lack of a time-lock mechanism for fee changes allows administrators to modify critical parameters instantly. This can be exploited in the following ways:

  • Front-Running Attacks: Malicious administrators or entities can monitor pending transactions and adjust fees to their advantage before the transactions are confirmed.

  • User Trust Erosion: Users may lose trust in the platform if they are subjected to unexpected fee hikes without any warning.

Proof of Concept (PoC) Test Code

  1. First create the mock: tests/mocks/ERC20Mock.sol with the following content:

    // SPDX-License-Identifier: UNLICENSED
    pragma solidity >=0.8.22;
    import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
    contract ERC20Mock is ERC20 {
    uint8 internal immutable DECIMAL;
    constructor(string memory name_, string memory symbol_, uint8 decimals_) ERC20(name_, symbol_) {
    DECIMAL = decimals_;
    }
    function decimals() public view override returns (uint8) {
    return DECIMAL;
    }
    function mint(address to, uint256 amount) public {
    _mint(to, amount);
    }
    }
  2. Create the mock file: tests/mocks/MockFlowNFTDescriptor.sol with the following content:

    // SPDX-License-Identifier: UNLICENSED
    pragma solidity >=0.8.22;
    import "../../src/interfaces/IFlowNFTDescriptor.sol";
    contract MockFlowNFTDescriptor is IFlowNFTDescriptor {
    function tokenURI(IERC721Metadata, uint256) external pure override returns (string memory) {
    return "Mock URI";
    }
    }
  3. Create the main test file: tests/SablierFlowFeeChangeTest.t.sol with the following content and run the test using command: forge test --mt testImmediateFeeChangeImpact -vvvv

    // SPDX-License-Identifier: UNLICENSED
    pragma solidity >=0.8.22;
    // Importing Forge's testing framework and necessary mock contracts
    import "forge-std/src/Test.sol";
    import "../src/SablierFlow.sol"; // Ensure the path is correct
    import "./mocks/ERC20Mock.sol";
    import "./mocks/MockFlowNFTDescriptor.sol";
    contract SablierFlowFeeChangeTest is Test {
    SablierFlow public sablierFlow;
    ERC20Mock public token;
    MockFlowNFTDescriptor public nftDescriptor;
    address public admin;
    address public user;
    uint256 public initialFee = 50; // 0.5%
    uint256 public newFee = 100; // 1.0%
    event TestStarted();
    event FeeUpdated(uint256 oldFee, uint256 newFee);
    event DepositMade(address indexed user, uint256 amount, uint256 fee);
    function setUp() public {
    emit TestStarted();
    admin = address(this);
    user = address(0xABC);
    token = new ERC20Mock("Mock Token", "MTK", 18);
    nftDescriptor = new MockFlowNFTDescriptor();
    sablierFlow = new SablierFlow(admin, nftDescriptor);
    _setProtocolFee(address(token), initialFee);
    emit FeeUpdated(0, initialFee);
    uint256 userInitialBalance = 1e24;
    token.mint(user, userInitialBalance);
    vm.startPrank(user);
    token.approve(address(sablierFlow), type(uint256).max);
    vm.stopPrank();
    }
    function _setProtocolFee(address tokenAddress, uint256 fee) internal {
    uint256 mappingSlot = 1;
    bytes32 slot = keccak256(abi.encodePacked(tokenAddress, bytes32(mappingSlot)));
    bytes32 feeValue = bytes32(fee);
    vm.store(address(sablierFlow), slot, feeValue);
    console.log("Protocol fee set:", fee);
    }
    function _getProtocolFee(address tokenAddress) internal view returns (uint256 fee) {
    uint256 mappingSlot = 1;
    bytes32 slot = keccak256(abi.encodePacked(tokenAddress, bytes32(mappingSlot)));
    bytes32 feeValue = vm.load(address(sablierFlow), slot);
    fee = uint256(feeValue);
    console.log("Protocol fee retrieved:", fee);
    }
    function testImmediateFeeChangeImpact() public {
    emit TestStarted();
    uint256 depositAmount = 1e18;
    vm.startPrank(admin);
    uint256 streamId = sablierFlow.create(
    user,
    address(0xDEF),
    ud21x18(1e18),
    IERC20(address(token)),
    true
    );
    vm.stopPrank();
    console.log("Stream created with ID:", streamId);
    vm.startPrank(user);
    uint256 userBalanceBefore = token.balanceOf(user);
    emit DepositMade(user, depositAmount, initialFee);
    console.log("User balance before deposit:", userBalanceBefore);
    uint256 expectedFee = (depositAmount * initialFee) / 10000;
    uint256 totalAmount = depositAmount + expectedFee;
    console.log("Expected fee for initial deposit:", expectedFee);
    console.log("Total amount to be deducted from user:", totalAmount);
    sablierFlow.depositViaBroker(
    streamId,
    uint128(depositAmount),
    user,
    address(0xDEF),
    Broker({ account: user, fee: UD60x18.wrap(initialFee) })
    );
    vm.stopPrank();
    uint256 userBalanceAfterDeposit = token.balanceOf(user);
    console.log("User balance after initial deposit:", userBalanceAfterDeposit);
    assertEq(
    userBalanceBefore - userBalanceAfterDeposit,
    totalAmount,
    "User should have paid the deposit amount plus initial fee"
    );
    _setProtocolFee(address(token), newFee);
    emit FeeUpdated(initialFee, newFee);
    uint256 currentFee = _getProtocolFee(address(token));
    assertEq(currentFee, newFee, "Protocol fee should have been updated to newFee");
    vm.startPrank(user);
    uint256 userBalanceBeforeSecondDeposit = token.balanceOf(user);
    console.log("User balance before second deposit:", userBalanceBeforeSecondDeposit);
    uint256 expectedFeeNew = (depositAmount * newFee) / 10000;
    uint256 totalAmountNew = depositAmount + expectedFeeNew;
    console.log("Expected fee for second deposit:", expectedFeeNew);
    console.log("Total amount to be deducted from user (new fee):", totalAmountNew);
    sablierFlow.depositViaBroker(
    streamId,
    uint128(depositAmount),
    user,
    address(0xDEF),
    Broker({ account: user, fee: UD60x18.wrap(initialFee) })
    );
    vm.stopPrank();
    uint256 userBalanceAfterSecondDeposit = token.balanceOf(user);
    console.log("User balance after second deposit:", userBalanceAfterSecondDeposit);
    assertEq(
    userBalanceBeforeSecondDeposit - userBalanceAfterSecondDeposit,
    totalAmountNew,
    "User should have paid the deposit amount plus new fee due to immediate fee change"
    );
    emit TestStarted();
    }
    }

Test Steps Breakdown

  1. Set Initial Fee and Deposit: The test begins by setting an initial fee (initialFee) and simulating a deposit by the user. This confirms that the user is charged according to the initial fee.

  2. Instant Fee Change: Right after the first deposit, the fee is increased to a new rate (newFee) by the administrator. This change takes effect immediately.

  3. Impact on Second Deposit: The user initiates a second deposit, unaware of the new fee rate. The second deposit charge confirms that the fee has changed and affects the user's costs.

Test Outcome

The failed assertion shows that the user was charged the updated fee unexpectedly, illustrating the vulnerability.

Impact

  • Financial Losses: Users may incur higher fees than expected, leading to direct financial losses.

  • Front-Running Risk: Malicious actors can exploit immediate fee changes to front-run transactions, manipulating the fee structure for personal gain.

  • Reduced Confidence: The platform may face reputational damage as users perceive it as unpredictable or insecure.

Tools Used

  • Code Review: Manual inspection of the SablierFlow smart contract code to identify governance and fee management mechanisms.

  • Security Analysis: Assessment of the contract's administrative functions and their impact on user interactions.

  • Testing Frameworks: Utilized tools like Foundry and Forge for simulating transactions and fee adjustments.

Recommendations

To mitigate the risks associated with immediate fee changes, the following measures are recommended:

Implement Time-Locked Fee Changes

Introduce a delay mechanism between proposing a fee change and its actual implementation. This can be achieved by:

  • Timelock Contract:

    • Deploy a timelock contract that queues fee changes.

    • Set a minimum delay period (e.g., 24 hours) before changes take effect.

    • Allow users to adjust their actions based on upcoming changes.

  • Governance Mechanism:

    • Establish a transparent governance process where fee changes are proposed and voted on by stakeholders.

    • Provide a public announcement of proposed changes, including rationale and impact assessment.

Example Implementation:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.22;
contract FeeManager {
uint256 public feePercentage;
uint256 public pendingFeePercentage;
uint256 public feeChangeTimestamp;
uint256 constant TIMELOCK_DURATION = 1 days;
event FeeChangeProposed(uint256 newFee, uint256 effectiveTime);
event FeeChanged(uint256 newFee);
// Propose a new fee percentage
function proposeFeeChange(uint256 _newFeePercentage) external onlyOwner {
pendingFeePercentage = _newFeePercentage;
feeChangeTimestamp = block.timestamp + TIMELOCK_DURATION;
emit FeeChangeProposed(_newFeePercentage, feeChangeTimestamp);
}
// Execute the fee change after timelock duration
function executeFeeChange() external {
require(block.timestamp >= feeChangeTimestamp, "Timelock not expired");
feePercentage = pendingFeePercentage;
emit FeeChanged(feePercentage);
}
}

Enhance Transparency

  • Public Notifications:

    • Announce upcoming fee changes through official channels.

    • Provide sufficient details and justifications for the changes.

  • User Alerts:

    • Implement in-app notifications or alerts for users about pending fee adjustments.

    • Allow users to opt-in for updates on governance proposals.

Monitor for Malicious Activity

  • Audit Administrative Actions:

    • Regularly review administrator transactions for suspicious fee changes.

    • Implement alert systems for sudden or significant adjustments.

  • Restrict Privileges:

    • Limit the number of accounts with the authority to propose or execute fee changes.

    • Use multi-signature wallets for critical administrative functions.

Educate Users

  • Documentation:

    • Update user guides to explain how fee changes are managed.

    • Clarify the timelines and processes involved in adjusting fees.

  • Community Engagement:

    • Encourage user participation in governance decisions.

    • Host discussions or forums to gather feedback on proposed changes.


By implementing these recommendations, the SablierFlow contract can enhance its security and user trust, ensuring that fee changes are managed transparently and predictably. This approach mitigates front-running risks and aligns the platform with best practices in smart contract governance.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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