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

Lack of ERC20 Transfer Validations and Malicious Token Risks

TxGuard

Summary

A vulnerability exists in the SablierFlow smart contract where it fails to validate the compliance of ERC20 tokens, particularly regarding the return values of transfer and transferFrom functions. This oversight can lead to unexpected behavior, including failed transactions, reentrancy attacks, and loss of funds when interacting with non-compliant or malicious ERC20 tokens.

Vulnerability Details

The SablierFlow contract interacts with ERC20 tokens by calling their transfer and transferFrom functions. It assumes that these functions always succeed and that the tokens comply fully with the ERC20 standard. However, not all tokens adhere strictly to the standard; some may not return a boolean value, always return true regardless of success, or even revert unexpectedly.

Additionally, malicious tokens can exploit this lack of validation to perform reentrancy attacks or other harmful actions during token transfers.

Proof of Concept (PoC):

Simply create the test file: tests/NonCompliantERC20.t.sol, and run the test using command: forge test --mt testNonCompliantTokenFailure -vvvv

// SPDX-License-Identifier: MIT
pragma solidity >=0.8.22;
​
import "forge-std/src/Test.sol";
import "../src/SablierFlow.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
​
contract MockERC20 is ERC20 {
constructor() ERC20("Mock Token", "MTK") {
_mint(msg.sender, 1e24); // Mint 1 million tokens to deployer
console.log("MockERC20 deployed. Initial supply minted to:", msg.sender);
}
}
​
contract NonCompliantERC20 is ERC20 {
constructor() ERC20("Non-Compliant Token", "NCT") {
_mint(msg.sender, 1e24); // Mint 1 million tokens to deployer
console.log("NonCompliantERC20 deployed. Initial supply minted to:", msg.sender);
}
​
// Overriding transfer to always revert
function transfer(address, uint256) public pure override returns (bool) {
console.log("NonCompliantERC20.transfer called, reverting...");
revert("Non-compliant token: transfer failed");
}
}
​
contract MaliciousERC20 is ERC20 {
SablierFlow public targetContract;
uint256 public streamId;
address public testAddress;
​
constructor(address _targetContract) ERC20("Malicious Token", "MLT") {
targetContract = SablierFlow(_targetContract);
_mint(msg.sender, 1e24); // Mint 1 million tokens to deployer
console.log("MaliciousERC20 deployed. Initial supply minted to:", msg.sender);
}
​
function setTestParams(uint256 _streamId, address _testAddress) external {
streamId = _streamId;
testAddress = _testAddress;
console.log("MaliciousERC20 test parameters set. Stream ID:", streamId, "Test Address:", testAddress);
}
​
// Overriding transfer to perform a reentrancy attack
function transfer(address, uint256) public override returns (bool) {
console.log("MaliciousERC20.transfer called, attempting reentrancy attack...");
try targetContract.withdraw(streamId, testAddress, 1e18) {
console.log("Reentrancy attack successful!");
return true;
} catch {
console.log("Reentrancy attack failed.");
revert("Malicious transfer triggered withdrawal attempt");
}
}
}
​
contract SablierFlowComprehensiveTest is Test {
SablierFlow sablierFlow;
MockERC20 token;
address sender = address(0x1);
address recipient = address(0x2);
uint256 streamId;
​
function setUp() public {
console.log("Setting up the test environment...");
​
// Deploy the compliant ERC20 token
token = new MockERC20();
console.log("MockERC20 token deployed at address:", address(token));
​
// Deploy the SablierFlow contract
sablierFlow = new SablierFlow(address(this), IFlowNFTDescriptor(address(0)));
console.log("SablierFlow contract deployed at address:", address(sablierFlow));
​
// Transfer tokens to sender
token.transfer(sender, 1e22); // 10,000 tokens
console.log("Transferred 10,000 tokens to sender at address:", sender);
​
// Sender approves SablierFlow to spend tokens
vm.startPrank(sender);
console.log("Prank started as sender:", sender);
​
token.approve(address(sablierFlow), type(uint256).max);
console.log("Sender approved SablierFlow to spend unlimited tokens");
​
vm.stopPrank();
console.log("Prank stopped as sender");
}
​
function testStandardTokenFlow() public {
console.log("\\\\nTesting standard token flow...");
​
vm.startPrank(sender);
console.log("Prank started as sender:", sender);
​
// Create a stream with the compliant token
streamId = sablierFlow.create(
sender,
recipient,
UD21x18.wrap(1e18), // 1 token per second
IERC20(address(token)),
true
);
console.log("Stream created with ID:", streamId);
​
// Deposit tokens into the stream
sablierFlow.deposit(streamId, 1e19, sender, recipient); // 10 tokens
console.log("Deposited 10 tokens into stream ID:", streamId);
​
vm.stopPrank();
console.log("Prank stopped as sender");
​
// Record recipient's balance before withdrawal
uint128 balanceBefore = uint128(token.balanceOf(recipient));
console.log("Recipient balance before withdrawal:", balanceBefore);
​
// Withdraw tokens from the stream
vm.prank(sender);
sablierFlow.withdraw(streamId, recipient, 5e18); // Withdraw 5 tokens
console.log("Withdrawn 5 tokens to recipient from stream ID:", streamId);
​
// Record recipient's balance after withdrawal
uint128 balanceAfter = uint128(token.balanceOf(recipient));
console.log("Recipient balance after withdrawal:", balanceAfter);
​
// Assert that the balance increased
assert(balanceAfter > balanceBefore);
console.log("Standard token flow test passed.");
}
​
function testNonCompliantTokenFailure() public {
console.log("\\\\nTesting non-compliant token failure...");
​
// Deploy the non-compliant token
NonCompliantERC20 nonCompliantToken = new NonCompliantERC20();
console.log("NonCompliantERC20 token deployed at address:", address(nonCompliantToken));
​
// Transfer tokens to sender
nonCompliantToken.transfer(sender, 1e22);
console.log("Transferred 10,000 non-compliant tokens to sender");
​
vm.startPrank(sender);
console.log("Prank started as sender:", sender);
​
// Sender approves SablierFlow to spend tokens
nonCompliantToken.approve(address(sablierFlow), type(uint256).max);
console.log("Sender approved SablierFlow to spend unlimited non-compliant tokens");
​
// Attempt to create a stream with the non-compliant token
try sablierFlow.create(
sender,
recipient,
UD21x18.wrap(1e18),
IERC20(address(nonCompliantToken)),
true
) {
console.log("Stream creation should have failed with non-compliant token, but it succeeded");
revert("Expected failure with non-compliant token");
} catch {
console.log("Stream creation failed as expected with non-compliant token");
}
​
vm.stopPrank();
console.log("Prank stopped as sender");
}
​
function testMaliciousTokenWithdrawalFailure() public {
console.log("\\\\nTesting malicious token withdrawal failure...");
​
// Deploy the malicious token
MaliciousERC20 maliciousToken = new MaliciousERC20(address(sablierFlow));
console.log("MaliciousERC20 token deployed at address:", address(maliciousToken));
​
// Transfer malicious tokens to sender
maliciousToken.transfer(sender, 1e22);
console.log("Transferred 10,000 malicious tokens to sender");
​
// Sender starts interacting
vm.startPrank(sender);
console.log("Prank started as sender:", sender);
​
// Create a stream with the compliant token
streamId = sablierFlow.create(
sender,
recipient,
UD21x18.wrap(1e18),
IERC20(address(token)),
true
);
console.log("Stream created with ID:", streamId);
​
// Deposit tokens into the stream
sablierFlow.deposit(streamId, 1e19, sender, recipient); // 10 tokens
console.log("Deposited 10 tokens into stream ID:", streamId);
​
// Set parameters for the malicious token to use in the attack
maliciousToken.setTestParams(streamId, address(this));
​
// Attempt to transfer malicious tokens, triggering reentrancy
try maliciousToken.transfer(address(0x3), 1e18) {
console.log("Malicious transfer should have failed, but it succeeded");
revert("Expected malicious transfer to trigger withdrawal failure");
} catch Error(string memory reason) {
console.log("Malicious transfer failed as expected with reason:", reason);
assertEq(reason, "Malicious transfer triggered withdrawal attempt");
}
​
vm.stopPrank();
console.log("Prank stopped as sender");
}
​
function testOverflowsWithHighRates() public {
console.log("\\\\nTesting overflows with high rates...");
​
vm.startPrank(sender);
console.log("Prank started as sender:", sender);
​
// Create a stream with the maximum possible rate
streamId = sablierFlow.create(
sender,
recipient,
UD21x18.wrap(type(uint128).max),
IERC20(address(token)),
true
);
console.log("Stream created with maximum rate per second");
​
// Attempt to deposit maximum tokens
try sablierFlow.deposit(streamId, type(uint128).max, sender, recipient) {
console.log("Deposit should have failed due to overflow, but it succeeded");
revert("Expected overflow handling on max rate deposit");
} catch {
console.log("Deposit failed as expected due to overflow");
}
​
// Verify that no tokens were incorrectly transferred
uint128 finalBalance = uint128(token.balanceOf(recipient));
console.log("Recipient final balance:", finalBalance);
​
assertEq(finalBalance, 0);
console.log("Overflow test passed, no incorrect fund transfer occurred.");
​
vm.stopPrank();
console.log("Prank stopped as sender");
}
}

Explanation:

  • testStandardTokenFlow: Confirms that the SablierFlow contract functions correctly with a compliant ERC20 token, showcasing normal behavior.

  • testNonCompliantTokenFailure: Demonstrates how the contract fails to handle a non-compliant token that reverts on transfer. Since the contract doesn't check for such behavior, it can cause unexpected failures.

  • testMaliciousTokenWithdrawalFailure: Shows how a malicious token can attempt a reentrancy attack during the transfer function, exploiting the lack of reentrancy guards in the SablierFlow contract.

  • testOverflowsWithHighRates: Tests the contract's ability to handle overflow conditions by using extremely high ratePerSecond values, which can cause arithmetic overflows if not properly handled.

Impact

  • Security Vulnerabilities:

    • Reentrancy Attacks: Malicious tokens can exploit the lack of reentrancy protection to perform unauthorized actions, potentially leading to the loss of funds.

    • Transaction Failures: Non-compliant tokens that do not adhere to the ERC20 standard can cause transactions to fail unexpectedly, disrupting the contract's functionality.

  • Financial Risks:

    • Loss of Funds: Users interacting with non-compliant or malicious tokens may experience loss of funds due to failed transactions or exploited vulnerabilities.

    • Contract Integrity: The overall trust in the SablierFlow contract may be compromised if it cannot securely handle different types of ERC20 tokens.

  • Operational Issues:

    • Denial of Service: Repeated failures or attacks could render the contract unusable, affecting all users.

    • Incompatibility: Limits the range of tokens that can safely interact with the contract, reducing its utility.

Tools Used

  • Foundry: A fast and modular Ethereum development toolkit used for compiling and testing the smart contracts.

  • Solidity Compiler (solc): Version 0.8.22, utilized for compiling the Solidity contracts.

  • Forge Std Library: Provides utilities for testing, including console.log for detailed output during test execution.

  • Console Logs (console.log): Used extensively in the PoC to trace execution flow and internal state for clarity.

Recommendations

  • Use SafeERC20 Library:

    • Integrate OpenZeppelin's SafeERC20 library to safely handle ERC20 token interactions, which checks the return values and handles non-standard tokens.

      import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
      ​
      contract SablierFlow {
      using SafeERC20 for IERC20;
      ​
      // Use safeTransfer and safeTransferFrom instead of transfer and transferFrom
      function _safeTokenTransfer(IERC20 token, address to, uint256 amount) internal {
      token.safeTransfer(to, amount);
      }
      ​
      function _safeTokenTransferFrom(IERC20 token, address from, address to, uint256 amount) internal {
      token.safeTransferFrom(from, to, amount);
      }
      }

  • Implement Reentrancy Guards:

    • Use OpenZeppelin's ReentrancyGuard to protect functions that involve token transfers and state changes.

      import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
      ​
      contract SablierFlow is ReentrancyGuard {
      // Apply the nonReentrant modifier to sensitive functions
      function withdraw(...) external nonReentrant {
      // Function implementation
      }
      }

  • Validate ERC20 Compliance:

    • Before interacting with a token, perform checks to ensure it adheres to the ERC20 standard.

    • Consider adding a token registry where tokens are verified before use.

  • Handle Return Values Explicitly:

    • Always check the return values of transfer and transferFrom and handle cases where tokens do not return a boolean.

      function safeTransfer(IERC20 token, address to, uint256 value) internal {
      bytes memory data = abi.encodeWithSelector(token.transfer.selector, to, value);
      (bool success, bytes memory returndata) = address(token).call(data);
      require(success, "Transfer failed");
      }

  • Enhance Testing Suite:

    • Expand unit tests to include various ERC20 token behaviors, including non-standard and malicious implementations.

    • Test edge cases like overflows, underflows, and reentrancy attacks thoroughly.

  • User Education:

    • Update documentation to inform users about the importance of using compliant tokens.

    • Provide guidelines or a list of recommended tokens known to be safe with the contract.

  • Regular Audits:

    • Conduct security audits to identify and fix vulnerabilities.

    • Stay updated with best practices in smart contract development.

By implementing these recommendations, the SablierFlow contract will become more secure and reliable, capable of handling interactions with a variety of ERC20 tokens safely. This will protect users from potential losses and enhance the contract's reputation in the ecosystem.

Note: Always ensure that any changes to the contract are thoroughly tested in a controlled environment before deployment to the mainnet.

Updates

Lead Judging Commences

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

Appeal created

drlaravel Submitter
10 months ago
inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 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.