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

Reentrancy Vulnerability in SablierFlow's External Calls

TxGuard

Summary

The SablierFlow contract is susceptible to reentrancy attacks due to unsecured external calls in functions like withdraw, deposit, refund, and depositViaBroker. These functions perform token transfers and interact with external contracts without implementing proper reentrancy guards. An attacker can exploit this vulnerability to manipulate the contract's state, potentially leading to the theft of funds or disruption of the contract's logic.

Vulnerability Details

Description

The SablierFlow contract facilitates token streaming between parties. Critical functions such as withdraw, deposit, refund, and depositViaBroker involve transferring tokens and updating internal states. However, these functions lack reentrancy protection, meaning they can be called repeatedly before the initial execution is complete.

An attacker can exploit this by crafting a malicious contract that re-enters the vulnerable function during an external call (e.g., token transfer). By doing so, the attacker can manipulate the contract's state in unintended ways, such as withdrawing more funds than allowed or corrupting the contract's balance tracking.

Proof of Concept

Below is a test contract demonstrating how a malicious broker can exploit the depositViaBroker function to perform a reentrancy attack. The test includes detailed comments and logs for clarity.

// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.8.22;
​
// Import the Foundry Test framework
import "forge-std/src/Test.sol";
// Import the SablierFlow contract to be tested
import "../src/SablierFlow.sol"; // Update with actual path
// Import mock ERC20 token for testing purposes
import "./mocks/ERC20Mock.sol";
// Import mock NFT descriptor (if needed)
import "./mocks/MockFlowNFTDescriptor.sol";
​
/**
* @title MaliciousBroker
* @dev Simulates a malicious broker that attempts to exploit reentrancy in the SablierFlow contract
*/
contract MaliciousBroker is Test {
SablierFlow public sablierFlow;
ERC20Mock public token;
​
// Constructor initializes the contract with references to SablierFlow and the token
constructor(SablierFlow _sablierFlow, ERC20Mock _token) {
sablierFlow = _sablierFlow;
token = _token;
}
​
// Fallback function to receive tokens (if needed)
receive() external payable {}
​
/**
* @dev Function to simulate a reentrancy attack during depositViaBroker
* @param streamId The ID of the stream to deposit into
* @param amount The amount of tokens to deposit
* @param sender The address of the sender
* @param recipient The address of the recipient
*/
function reenterDeposit(uint256 streamId, uint128 amount, address sender, address recipient) external {
// Step 1: Mint tokens to this contract to fund the deposit
token.mint(address(this), amount);
emit log_named_uint("MaliciousBroker token balance after mint", token.balanceOf(address(this)));
​
// Step 2: Approve SablierFlow to spend tokens on behalf of MaliciousBroker
token.approve(address(sablierFlow), amount);
emit log("MaliciousBroker approved SablierFlow to spend tokens.");
​
// Step 3: Call depositViaBroker, attempting to re-enter during the external call
sablierFlow.depositViaBroker(
streamId,
amount,
sender,
recipient,
Broker({
account: address(this),
fee: UD60x18.wrap(0) // Correctly wrapped using UD60x18
})
);
emit log("MaliciousBroker called depositViaBroker.");
}
}
​
/**
* @title SablierFlowReentrancyTest
* @dev Test contract to demonstrate the reentrancy vulnerability in SablierFlow
*/
contract SablierFlowReentrancyTest is Test {
SablierFlow public sablierFlow;
ERC20Mock public token;
MockFlowNFTDescriptor public nftDescriptor;
address public sender;
address public recipient;
MaliciousBroker public maliciousBroker;
​
/**
* @dev Sets up the test environment, deploying contracts and setting initial balances
*/
function setUp() public {
// Deploy the mock ERC20 token
token = new ERC20Mock("Mock Token", "MTK", 18);
emit log_named_address("ERC20Mock deployed at", address(token));
​
// Deploy the mock NFT descriptor
nftDescriptor = new MockFlowNFTDescriptor();
emit log_named_address("MockFlowNFTDescriptor deployed at", address(nftDescriptor));
​
// Deploy the SablierFlow contract
sablierFlow = new SablierFlow(address(this), nftDescriptor);
emit log_named_address("SablierFlow deployed at", address(sablierFlow));
​
// Set the sender and recipient addresses
sender = address(0x123);
recipient = address(0x456);
​
// Mint tokens to the sender
token.mint(sender, 1e24); // Mint 1,000,000 tokens (assuming 18 decimals)
emit log_named_uint("Sender's initial token balance", token.balanceOf(sender));
​
// Sender approves SablierFlow to spend their tokens
vm.startPrank(sender); // Impersonate the sender
token.approve(address(sablierFlow), type(uint256).max);
emit log("Sender approved SablierFlow to spend tokens.");
vm.stopPrank(); // Stop impersonating
​
// Deploy the MaliciousBroker contract
maliciousBroker = new MaliciousBroker(sablierFlow, token);
emit log_named_address("MaliciousBroker deployed at", address(maliciousBroker));
}
​
/**
* @dev Tests the reentrancy vulnerability in depositViaBroker
*/
function testDepositViaBrokerReentrancy() public {
emit log("=== Starting testDepositViaBrokerReentrancy ===");
​
// Step 1: Sender creates a new stream to the recipient
vm.startPrank(sender); // Impersonate the sender
uint256 streamId = sablierFlow.create(
sender,
recipient,
UD21x18.wrap(1e18), // ratePerSecond is 1 token per second
token,
true // Transferable
);
emit log_named_uint("Stream created with ID", streamId);
vm.stopPrank(); // Stop impersonating
​
// Step 2: Attempt to exploit reentrancy in depositViaBroker via MaliciousBroker
vm.startPrank(sender); // Impersonate the sender
emit log("Sender attempting to deposit via MaliciousBroker to exploit reentrancy.");
​
// Expect the transaction to revert due to reentrancy protection
vm.expectRevert(); // Expect any revert
maliciousBroker.reenterDeposit(streamId, 1e18, sender, recipient);
emit log("MaliciousBroker attempted reentrant deposit.");
​
vm.stopPrank(); // Stop impersonating
​
emit log("=== Completed testDepositViaBrokerReentrancy ===");
}
}
​

Notes:

  • The test demonstrates how a malicious broker can attempt to exploit the depositViaBroker function.

  • Extensive comments and logs are added for clarity.

  • The vm.expectRevert() function is used to assert that the transaction should fail if reentrancy protection is correctly implemented.

Impact

  • Financial Loss: An attacker exploiting this vulnerability can manipulate the contract's state to withdraw more funds than allowed, leading to potential loss of all contract funds.

  • Contract Integrity: Reentrancy attacks can disrupt the logical flow of the contract, causing unexpected behaviors and undermining trust.

  • Historical Precedence: Similar vulnerabilities have led to significant losses in the past, such as The DAO hack, emphasizing the severity of this issue.

Tools Used

  • Solidity Compiler (pragma solidity >=0.8.22): For writing and compiling smart contracts.

  • Foundry (Forge): A testing framework for Ethereum smart contracts.

  • ERC20Mock and MockFlowNFTDescriptor: Mock contracts for testing purposes.

  • MaliciousBroker Contract: A custom contract designed to simulate a reentrancy attack.

Recommendations

  1. Implement Reentrancy Guards:

    • Use OpenZeppelin's ReentrancyGuard contract or a similar mechanism.

    • Apply the nonReentrant modifier to all functions that perform external calls and update critical state variables.

  2. Check-Effects-Interactions Pattern:

    • Ensure that all state changes occur before external calls.

    • This minimizes the window of opportunity for reentrancy attacks.

  3. Use the Checks Modifier:

    • Implement custom modifiers that enforce necessary preconditions before function execution.

  4. Audit External Calls:

    • Review all functions that interact with external contracts or perform token transfers.

    • Ensure that they are secured against reentrancy and other external threats.

  5. Limit External Contract Interaction:

    • Reduce reliance on external contracts where possible.

    • Favor internal logic and trusted contracts.

  6. Testing and Validation:

    • Write comprehensive tests simulating various attack vectors, including reentrancy.

    • Regularly update tests to cover new threats as they emerge.

  7. Stay Updated on Best Practices:

    • Follow the latest security guidelines from reputable sources like OpenZeppelin.

    • Keep abreast of known vulnerabilities and recommended mitigations.

Conclusion

The lack of reentrancy protection in critical functions of the SablierFlow contract poses a significant security risk. Implementing the recommended safeguards is essential to protect the contract from potential attacks, ensure the safety of user funds, and maintain the integrity of the system.

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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