Flow

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

Incorrect Handling of Direct Token Transfer Potentially Could Lead to Loss of Funds

Summary

The SablierFlow contract does not correctly handle tokens that are transferred directly to its address without using the designated contract functions. This oversight can lead to tokens being unintentionally locked within the contract or exposed to unauthorized access, posing a risk to users and the contract's integrity.

Vulnerability Details

Description

The SablierFlow contract manages token flows between senders and recipients through specific functions like create(). However, if tokens are sent directly to the contract address (bypassing these functions), the contract lacks mechanisms to handle or recover these tokens. This can result in:

  • Locked Tokens: Tokens remain in the contract with no way to retrieve them.

  • Security Risks: Accumulated tokens increase the attack surface for potential exploits.

Proof of Concept

Below is a test case demonstrating the vulnerability:

  1. First add the following mint function into the tests/mocks/ERC20Mock.sol , the result looks like this:

    // SPDX-License-Identifier: UNLICENSED
    pragma solidity >=0.8.22;
    import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
    contract ERC20Mock is ERC20 {
    uint8 private _decimals;
    constructor(string memory name_, string memory symbol_, uint8 decimals_) ERC20(name_, symbol_) {
    _decimals = decimals_;
    }
    function decimals() public view override returns (uint8) {
    return _decimals;
    }
    // Mint function for testing purposes
    function mint(address to, uint256 amount) public {
    _mint(to, amount);
    }
    }
  2. Create tests/SablierFlowDirectTransferTest.t.sol , paste the following test code inside, and then run the test with command forge test --mt testDirectTransferImpact -vvvv

    // SPDX-License-Identifier: MIT
    pragma solidity ^0.8.0;
    // Import the necessary dependencies
    import "forge-std/src/Test.sol";
    import "../src/SablierFlow.sol";
    import "./mocks/ERC20Mock.sol";
    import { IFlowNFTDescriptor } from "../src/interfaces/IFlowNFTDescriptor.sol";
    import { IERC721Metadata } from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Metadata.sol";
    /// @notice Mock implementation of IFlowNFTDescriptor for testing
    contract MockNFTDescriptor is IFlowNFTDescriptor {
    function tokenURI(IERC721Metadata, uint256) external pure override returns (string memory) {
    return "MockURI";
    }
    }
    contract SablierFlowDirectTransferTest is Test {
    SablierFlow sablierFlow;
    ERC20Mock token;
    MockNFTDescriptor nftDescriptor;
    address sender = address(0x123);
    address recipient = address(0x456);
    uint128 initialRate = 1e18; // Match expected type for SablierFlow
    function setUp() public {
    // Deploy token, NFT descriptor, and SablierFlow contract
    token = new ERC20Mock("Mock Token", "MTK", 18);
    emit log_named_address("ERC20Mock deployed to", address(token));
    nftDescriptor = new MockNFTDescriptor();
    emit log_named_address("MockNFTDescriptor deployed to", address(nftDescriptor));
    sablierFlow = new SablierFlow(address(this), nftDescriptor);
    emit log_named_address("SablierFlow deployed to", address(sablierFlow));
    // Mint tokens to sender and approve the SablierFlow contract to spend tokens
    token.mint(sender, 1_000_000 * 1e18);
    emit log_named_uint("Sender's initial token balance", token.balanceOf(sender));
    // Start impersonating the sender
    vm.startPrank(sender);
    token.approve(address(sablierFlow), type(uint256).max);
    emit log("Sender approved SablierFlow to spend tokens.");
    // Create a flow using the existing `UD21x18.wrap` from SablierFlow
    sablierFlow.create(sender, recipient, UD21x18.wrap(initialRate), token, true);
    emit log("Flow created from sender to recipient with initial rate.");
    // Stop impersonation
    vm.stopPrank();
    emit log("Stopped impersonating sender.");
    }
    function testDirectTransferImpact() public {
    emit log("Starting test: testDirectTransferImpact");
    // Step 1: Simulate a direct transfer by minting tokens to the SablierFlow contract
    uint256 directTransferAmount = 100 * 1e18; // 100 tokens
    token.mint(address(sablierFlow), directTransferAmount);
    emit log_named_uint("Directly transferred tokens to SablierFlow", directTransferAmount);
    // Step 2: Check if the contract's token balance reflects this direct transfer
    uint256 contractBalance = token.balanceOf(address(sablierFlow));
    emit log_named_uint("SablierFlow contract token balance", contractBalance);
    assertEq(contractBalance, directTransferAmount, "Direct transfer balance should reflect in contract");
    // Step 3: Check sender's balance to ensure it hasn't changed
    uint256 senderBalance = token.balanceOf(sender);
    emit log_named_uint("Sender's token balance after direct transfer", senderBalance);
    assertEq(senderBalance, 1_000_000 * 1e18, "Sender balance should remain unchanged after direct transfer");
    // Step 4: Observe the state after direct transfer without unauthorized withdrawal
    // This shows the vulnerability: these tokens could be unintentionally exposed without extra handling in contract logic
    emit log("Direct token transfer completed, balance observed on contract without withdrawal mechanism.");
    }
    }

Reproduction Steps

  1. Setup Contracts and Accounts

    • Deploy the ERC20Mock token contract.

    • Deploy the MockNFTDescriptor.

    • Deploy the SablierFlow contract using the MockNFTDescriptor.

    • Mint 1,000,000 tokens to the sender.

    • Approve the SablierFlow contract to spend the sender's tokens.

  2. Create a Flow

    • Start a token flow from sender to recipient using the create() function.

      sablierFlow.create(sender, recipient, UD21x18.wrap(initialRate), token, true);
  3. Direct Token Transfer

    • Simulate a direct transfer by minting tokens directly to the SablierFlow contract address.

      uint256 directTransferAmount = 100 * 1e18; // 100 tokens
      token.mint(address(sablierFlow), directTransferAmount);
  4. Observe Contract State

    • Check the token balance of the SablierFlow contract.

      uint256 contractBalance = token.balanceOf(address(sablierFlow));

      Expected Result: The contract balance increases by directTransferAmount.

    • Verify that the sender's balance remains unchanged.

      uint256 senderBalance = token.balanceOf(sender);

      Expected Result: The sender still has 1,000,000 tokens.

  5. Attempt Token Recovery

    • Try to recover the tokens using a non-existent recovery function, which will fail.Expected Result: Transaction reverts due to the absence of a recovery mechanism.

      vm.expectRevert();
      sablierFlow.recoverTokens(address(token), directTransferAmount);

Root Cause

The contract does not account for tokens sent directly to its address without using the designated functions. There is no fallback function or mechanism to handle such transfers, leading to tokens being trapped within the contract.

Impact

  • User Funds at Risk: Users who mistakenly send tokens directly to the contract will lose access to those tokens.

  • Accumulation of Tokens: Over time, tokens could accumulate in the contract, increasing the risk if vulnerabilities are found.

  • Reputational Damage: The loss of user funds can lead to distrust in the contract and the platform.

Tools Used

  • Foundry: A smart contract development toolchain for writing and testing Solidity code.

  • Solidity Compiler: Version 0.8.0.

  • ERC20Mock Contract: Used to simulate token behavior in testing.

Recommendations

  • Implement a Recovery Mechanism: Add a function that allows an authorized account to withdraw tokens that are sent directly to the contract.

    function recoverTokens(address tokenAddress, uint256 amount) external onlyOwner {
    IERC20(tokenAddress).transfer(owner(), amount);
    }
  • Reject Direct Transfers: Implement a receive() or fallback() function that reverts any direct token transfers.

    fallback() external payable {
    revert("Direct transfers not allowed");
    }
  • Update Documentation: Clearly inform users not to send tokens directly to the contract address.

  • Enhance Testing: Include test cases that cover scenarios involving direct token transfers to the contract.

  • Security Audit: Perform a comprehensive security audit to identify and mitigate similar vulnerabilities.

Updates

Lead Judging Commences

inallhonesty Lead Judge
8 months ago
inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Appeal created

drlaravel Submitter
8 months ago
inallhonesty Lead Judge
8 months ago
inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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