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

[H-1] _deposit :: SablierFlow has both stream balance and aggregate balance update before safeTransfer being implemted

oluwaseyisekoni

Description

function _deposit(uint256 streamId, uint128 amount) internal {
// Check: the deposit amount is not zero.
if (amount == 0) {
revert Errors.SablierFlow_DepositAmountZero(streamId);
}
​
IERC20 token = _streams[streamId].token;
​
// Effect: update the stream balance.
_streams[streamId].balance += amount;
​
unchecked {
// Effect: update the aggregate balance.
aggregateBalance[token] += amount;
}
​
// Interaction: transfer the amount.
token.safeTransferFrom({ from: msg.sender, to: address(this), value: amount });
​
// Log the deposit.
emit ISablierFlow.DepositFlowStream({ streamId: streamId, funder: msg.sender, amount: amount });
}

The function _depositis defected due to the fact _streams[streamId].balanceand aggregateBalance[token]are updated before token.safeTransferFrom

Impact

1) Failed Transfer Leading to Inconsistent State:

If the safeTransfer fails (e.g., due to insufficient balance, incorrect approval, or a transfer rejection by the token contract), the function would typically revert. However, if the state has already been updated, it could lead to an inconsistent state, as the contract would have already recorded the deposit even though the transfer did not actually occur.

In this case, the streams' balance would be incremented without actually paying off the debt, resulting in a situation where the contract thinks the debt has been reduced when it hasn't.

2) Reentrancy Exploit Scenario:

If the token contract is malicious or has a fallback that calls back into the contract, an attacker could exploit this to re-enter the deposit function and manipulate the state in unintended ways

3) Debtor could use tokens with:

  • Blacklisting

  • Pausing

  • Rebasing mechanisms

Proof of Concepts

1)

contract DebtorAttack1 {
VulnerableStreamDeposit private stream;
IERC20 private token;
uint256 private streamId;
function attack(uint256 _streamId, uint128 amount) external {
// Debtor has insufficient balance
// but stream balance is still updated before revert
stream.deposit(_streamId, amount);
// Transaction reverts but could cause issues with:
// 1. stream balance
// 2. aggregaet[token] balancee
}
}
​

2)

​
contract MaliciousToken is IERC20 {
mapping(address => uint256) private _balances;
VulnerableStreamDeposit private stream;
function transferFrom(
address from,
address to,
uint256 amount
) external returns (bool) {
// Get stream balance credited
// Then revert for any reason:
// 1. Blacklist
// 2. Pausing
// 3. Custom transfer logic
revert("Transfer blocked");
}
}

3)

contract ReentrantToken is IERC20 {
VulnerableStreamDeposit private stream;
uint256 private streamId;
function transferFrom(
address from,
address to,
uint256 amount
) external returns (bool) {
// First get original deposit credited
// Then do flash loan attack:
if (to == address(stream)) {
// 1. Flash mint tokens
_mint(from, amount);
// 2. Make another deposit
stream.deposit(streamId, uint128(amount));
// 3. Burn flash minted tokens
_burn(from, amount);
}
return true;
}
}

Recommended mitigation

  1. Transfer before state updates

  2. Verify actual received amounts

  3. Update state based on verified amounts

    function _deposit(uint256 streamId, uint128 amount) internal {
    if (amount == 0) revert("Amount zero");
    IERC20 token = _streams[streamId]token;
    // Get initial balance
    uint256 balanceBefore = token.balanceOf(address(this));
    // Transfer first
    token.safeTransferFrom(msg.sender, address(this), amount);
    // Verify actual amount received
    uint256 received = token.balanceOf(address(this)) - balanceBefore;
    if (received != amount) revert("Invalid transfer amount");
    // Update state after successful transfer
    stream.balance += amount;
    aggregateBalance[token] += amount;
    emit DepositFlowStream(streamId, msg.sender, amount);
    }
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
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.