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

Division by Zero Vulnerability in SablierFlow.sol Leading to Panic Error

mrxsnowden

Summary

The SablierFlow contract contains a potential division-by-zero vulnerability in the _calculateDepletionTime function. This vulnerability could lead to a panic error when the ratePerSecond variable is zero, causing the contract to revert unexpectedly.

Vulnerability Details

The issue is in the following function, which calculates the depletion time based on the contract's solvency rate :
https://github.com/Cyfrin/2024-10-sablier/blob/8a2eac7a916080f2022527408b004578b21c51d0/src/SablierFlow.sol#L93

If ratePerSecond is zero, dividing solvencyAmount by ratePerSecond results in a division-by-zero panic. This leads to an immediate halt in the execution of the function and reverts the transaction, causing instability for any users or external contracts relying on this function.

POC

Add the following test to tests folder :

// SPDX-License-Identifier: UNLICENSED
pragma solidity >=0.8.22;
​
import "forge-std/src/Test.sol";
import "../src/SablierFlow.sol";
import "../src/interfaces/IFlowNFTDescriptor.sol";
​
// Mock NFT Descriptor contract
contract MockNFTDescriptor is IFlowNFTDescriptor {
function tokenURI(IERC721Metadata sablierFlow, uint256 streamId) external view override returns (string memory) {
return "https://example.com/tokenURI"; // Return a mock URI for testing
}
}
​
​
contract SablierFlowTest is Test {
SablierFlow sablierFlow;
​
function setUp() public {
address initialAdmin = address(this); // Use the test contract as the admin
IFlowNFTDescriptor initialNFTDescriptor = new MockNFTDescriptor(); // Mock descriptor
​
// Deploy an instance of the SablierFlow contract with the required arguments
sablierFlow = new SablierFlow(initialAdmin, initialNFTDescriptor);
}
​
function testDivisionByZero() public {
// Setup: Initialize variables to simulate the scenario
uint256 streamId = 1; // Assuming streamId 1 is valid for testing
uint256 balanceScaled = 1000; // Example balance in scaled units
uint256 snapshotDebtScaled = 500; // Example debt in scaled units
uint256 oneMVTScaled = 100; // Example MVT (Minimum Viable Token) scaled units
// Scenario: Set ratePerSecond to zero
uint256 ratePerSecond = 0;
​
// Expect a revert due to division by zero
vm.expectRevert("division by zero"); // Adjust the error message based on actual revert message
// Call the function with the setup values
unchecked {
uint256 solvencyAmount = balanceScaled - snapshotDebtScaled + oneMVTScaled;
uint256 solvencyPeriod = solvencyAmount / ratePerSecond; // Should trigger division-by-zero
}
}
}

Conditions

The division-by-zero occurs when ratePerSecond is equal to zero, which can happen under certain configurations or states of the contract. This error has been reproduced in a unit test (testDivisionByZero) that intentionally sets ratePerSecond to zero.

Impact

This vulnerability has a medium impact as it could lead to:

  1. Unexpected Reverts: The division-by-zero panic causes the transaction to revert. This affects the contract's usability and can disrupt functionality for users and dependent contracts.

  2. Potential Denial of Service: If ratePerSecond can be manipulated externally, this could be exploited to cause repeated reverts, creating a denial of service.

Tools Used

1- Foundry for setting up and running the unit test.

2- Manual Code Review to locate the vulnerable line of code.

Recommendations

1- Fallback Logic: If ratePerSecond being zero is an expected but rare case, implement fallback logic to handle this scenario safely.
2-Check for Zero: Before performing division, add a check to ensure ratePerSecond is non-zero:

// Add the require statement here
+ require(ratePerSecond != 0, "Division by zero: ratePerSecond must be non-zero");
​
uint256 solvencyPeriod = solvencyAmount / ratePerSecond;
Updates

Lead Judging Commences

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

Support

FAQs

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