Flow

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

Dangerous Strict Equality in _withdraw function at (src/SablierFlow.sol#866) : assert(bool)(totalDebt - _totalDebtOf(streamId) == balance - _streams[streamId].balance) (src/SablierFlow.sol#866)

Summary : : The _withdraw function in the SablierFlow contract uses a strict equality comparison (==) to check if the difference between totalDebt and _totalDebtOf(streamId) is equal to the difference between balance and _streams[streamId].balance. This can potentially lead to unexpected behavior if there are precision issues or if the values are very close but not exactly equal.

Vulnerability Details : Dangerous Strict Equality

Let's break down the code snippet:

assert(bool)(totalDebt - _totalDebtOf(streamId) == balance - _streams[streamId].balance);

Here's what's happening:

  • totalDebt is a variable that stores the total debt.

  • _totalDebtOf(streamId) is a function that returns the total debt of a specific stream with ID streamId.

  • balance is a variable that stores the balance.

  • _streams[streamId].balance is a variable that stores the balance of a specific stream with ID streamId.

The code is checking if the difference between totalDebt and _totalDebtOf(streamId) is equal to the difference between balance and _streams[streamId].balance.

Here's a step-by-step breakdown:

  1. totalDebt - _totalDebtOf(streamId): This calculates the difference between the total debt and the total debt of the specific stream.

  2. balance - _streams[streamId].balance: This calculates the difference between the balance and the balance of the specific stream.

  3. ==: This is a strict equality comparison operator that checks if the two values are exactly equal.

The issue here is that the == operator is used to compare two values that may not be exactly equal due to precision issues or small differences. This can lead to unexpected behavior, such as the contract reverting when it shouldn't, or not reverting when it should.

For example, let's say totalDebt is 1000 and _totalDebtOf(streamId) is 999.99. The difference between the two values would be 0.01. Similarly, let's say balance is 900 and _streams[streamId].balance is 899.99. The difference between the two values would be 0.01.

In this case, the == operator would return false, even though the differences between the values are very small. This could lead to unexpected behavior, such as the contract reverting when it shouldn't.

Impact : This issue can lead to unexpected behavior, such as:

  • Contract reverting when it shouldn't: If the comparison returns false due to a small difference between the values, the contract may revert even though the condition is actually true.

  • Contract not reverting when it should: If the comparison returns true due to a small difference between the values, the contract may not revert even though the condition is actually false.

This can lead to a range of problems, including:

  • Incorrect behavior: The contract may behave in unexpected ways, leading to incorrect results or unintended consequences.

  • Security vulnerabilities: In some cases, the contract may be vulnerable to attacks or exploits that take advantage of the incorrect behavior.

  • User frustration: Users may experience unexpected behavior or errors, leading to frustration and a negative experience.

Proof of Concept Code :

contract ExploitContract {
SablierFlow public sablierFlow;
constructor(address sablierFlowAddress) public {
sablierFlow = SablierFlow(sablierFlowAddress);
}
function exploit() public {
// Call the _withdraw function with a specific streamId and amount
uint256 streamId = 1;
address to = address(this);
uint128 amount = 100;
// Set the totalDebt and balance variables to specific values
uint256 totalDebt = 1000;
uint256 balance = 900;
// Call the _withdraw function
sablierFlow._withdraw(streamId, to, amount);
// Check if the contract reverted
require(!sablierFlow.reverted(), "Contract reverted unexpectedly");
// Check if the totalDebt and balance variables were updated correctly
require(sablierFlow.totalDebt() == totalDebt - amount, "Total debt not updated correctly");
require(sablierFlow.balance() == balance - amount, "Balance not updated correctly");
}

Let's break down the code snippet:

contract ExploitContract {
SablierFlow public sablierFlow;

This defines a new contract called ExploitContract that has a public variable sablierFlow of type SablierFlow.

Constructor:

constructor(address sablierFlowAddress) public {
sablierFlow = SablierFlow(sablierFlowAddress);
}

This is the constructor function that is called when the contract is deployed. It takes an address parameter sablierFlowAddress and uses it to initialize the sablierFlow variable.

Exploit Function:

function exploit() public {
// ...
}

This is the main function that is designed to exploit the vulnerability in the SablierFlow contract.

Setup:

uint256 streamId = 1;
address to = address(this);
uint128 amount = 100;
uint256 totalDebt = 1000;
uint256 balance = 900;

This sets up some variables that will be used to exploit the vulnerability. streamId is set to 1, to is set to the address of the current contract, and amount is set to 100. totalDebt and balance are set to 1000 and 900, respectively.

Calling the _withdraw Function:

sablierFlow._withdraw(streamId, to, amount);

This calls the _withdraw function on the SablierFlow contract, passing in the streamId, to, and amount variables.

Checking for Reversion:

require(!sablierFlow.reverted(), "Contract reverted unexpectedly");

This checks if the SablierFlow contract reverted after calling the _withdraw function. If it did, the require statement will throw an exception.

Checking for Correct Updates:

require(sablierFlow.totalDebt() == totalDebt - amount, "Total debt not updated correctly");
require(sablierFlow.balance() == balance - amount, "Balance not updated correctly");

This checks if the totalDebt and balance variables were updated correctly after calling the _withdraw function. If they were not, the require statements will throw exceptions.

Overall, this contract is designed to exploit a vulnerability in the SablierFlow contract by calling the _withdraw function with specific inputs and checking if the contract behaves correctly.

Tools Used : VS code

Recommendations : To fix this issue, it's recommended to use a more robust comparison method, such as >= or <=, to account for potential precision issues or small differences between the values.

Here's an example of how to modify the code:

assert(bool)(totalDebt - _totalDebtOf(streamId) >= balance - _streams[streamId].balance - 1);

In this example, we've replaced the == operator with >=, and added a small tolerance value of 1 to account for potential precision issues or small differences between the values.

By using a more robust comparison method, we can ensure that the contract behaves correctly even in the presence of small differences between the values.

Benefits:

Using a more robust comparison method provides several benefits, including:

  • Improved accuracy: The contract will behave more accurately, even in the presence of small differences between the values.

  • Reduced risk: The contract will be less vulnerable to attacks or exploits that take advantage of incorrect behavior.

  • Better user experience: Users will experience fewer errors and unexpected behavior, leading to a more positive experience.

Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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