Summary
The depletion time, as defined in the code comment of the depletionTimeOf function, is the UNIX timestamp when the
total debt exceeds the stream balance by one token (mvt). However, the ongoing debt contributing to the total debt was
not included in the calculation of the solvencyAmount. As a result, the solvencyPeriod derived from solvencyAmount
was inaccurate. This led to an overestimation of the depletion time, that give wrong impression that the stream fund
balance can still last to the calculated timestamp but in fact it depletes more quickly than anticipated.
https://github.com/Cyfrin/2024-10-sablier/blob/8a2eac7a916080f2022527408b004578b21c51d0/src/SablierFlow.sol#L87-L93
Vulnerability Details
In SablierFlow::depletionTimeOf function, when checking if total debt exceeds balance, the _ongoingDebtScaledOf was
factored in. If total debts do exceed balance, then the depletion time is set as 0.
if (snapshotDebtScaled + _ongoingDebtScaledOf(streamId) >= balanceScaled + oneMVTScaled) {
return 0;
}
In the past audit, finding 3.3.4 , it was reported
that if (snapshotDebt + _ongoingDebtOf(streamId) > balance) {} was fixed at
PR296. Therefore it is assumed that the above line of code was
supposed to be > instead of >=.
However, at subsequents steps to calculate the depletion time, _ongoingDebtOf was not factored in despite code
comments emphasizing the intention to find the lowest timestamp at which the total debt exceeds the stream balance.
<@@>!
unchecked {
<@@>! uint256 solvencyAmount = balanceScaled - snapshotDebtScaled + oneMVTScaled;
uint256 solvencyPeriod = solvencyAmount / ratePerSecond;
if (solvencyAmount % ratePerSecond == 0) {
depletionTime = _streams[streamId].snapshotTime + solvencyPeriod;
}
else {
depletionTime = _streams[streamId].snapshotTime + solvencyPeriod + 1;
}
}
The solvencyAmount was calculated with only snapshotDebtScaled was minus off from the balanceScaled. The
_ongoingDebtOf which also contributing to the total debts was not factored in in the solvencyAmount calculation. As
a result, the solvencyPeriod derived from solvencyAmount was inaccurate. This led to an overestimation of the
subsequent calculation on depletion time , which wrongly gives an impression of longer time but in reality the stream
fund could deplete more quickly than anticipated depending on the timestamp the function is called after the previous
snapshot.
Proof of Concept:
Step 1: Add the following test case to tests\integration\concrete\depletion-time-of\depletionTimeOf.t.sol
function test_audit_depletionTimeIssue()
external
givenNotNull
givenNotPaused
givenBalanceNotZero
givenNoUncoveredDebt
{
flow.deposit(defaultStreamId, DEPOSIT_AMOUNT_6D, users.sender, users.recipient);
uint40 depletionTime = uint40(flow.depletionTimeOf(defaultStreamId));
console.log("depletionTime: ", depletionTime);
vm.warp({ newTimestamp: getBlockTimestamp() + 100 days });
uint40 depletionTimeAfter100days = uint40(flow.depletionTimeOf(defaultStreamId));
console.log("depletionTimeAfter100days: ", depletionTimeAfter100days);
assertLt(depletionTimeAfter100days, depletionTime, "depletion time after 100 days");
}
Step 2: Run the test forge test --match-test test_audit_depletionTimeIssue --nmt testFork --via-ir -vvv
forge test --match-test test_audit_depletionTimeIssue --nmt testFork --via-ir -vvv
[⠊] Compiling...
[⠑] Compiling 4 files with Solc 0.8.26
[⠘] Solc 0.8.26 finished in 9.05s
Compiler run successful!
Ran 1 test for tests/integration/concrete/depletion-time-of/depletionTimeOf.t.sol:DepletionTimeOf_Integration_Concrete_Test
[FAIL: depletion time after 100 days: 1827740801 >= 1827740801] test_audit_depletionTimeIssue() (gas: 245878)
Logs:
depletionTime: 1827740801
depletionTimeAfter100days: 1827740801
The test failed the assertion with depletion time before and after 100 days remain the same at 1827740801 unix
timestamp. This indicates that the existing calculation of depletion time in depletionTimeOf is incorrect and does not
take the ongoing debts into consideration.
Impact
The calculated depletion time does not accurately reflect when the stream fund will deplete, particularly in the
presence of ongoing debts. As a result, it gives wrong impression that the stream fund balance can still last to the
calculated timestamp but in fact it depletes more quickly than anticipated with uncovered debts are arising sooner than
the calculated timestamp indicates.
Tools Used
Manual review with test
Recommendations
Factor in the ongoing debts in the calculation of solvencyAmount in depletionTimeOf function as below:
function depletionTimeOf(uint256 streamId)
external
view
override
notNull(streamId)
notPaused(streamId)
returns (uint256 depletionTime)
{
uint128 balance = _streams[streamId].balance;
// If the stream balance is zero, return zero.
if (balance == 0) {
return 0;
}
uint8 tokenDecimals = _streams[streamId].tokenDecimals;
uint256 balanceScaled = Helpers.scaleAmount({ amount: balance, decimals: tokenDecimals });
uint256 snapshotDebtScaled = _streams[streamId].snapshotDebtScaled;
// MVT represents Minimum Value Transferable, the smallest amount of token that can be transferred, which is
// always 1 in token's decimal.
uint256 oneMVTScaled = Helpers.scaleAmount({ amount: 1, decimals: tokenDecimals });
// If the total debt exceeds balance, return zero.
if (snapshotDebtScaled + _ongoingDebtScaledOf(streamId) >= balanceScaled + oneMVTScaled) {
return 0;
}
uint256 ratePerSecond = _streams[streamId].ratePerSecond.unwrap();
// Depletion time is defined as the UNIX timestamp at which the total debt exceeds stream balance by 1 unit of
// token (mvt). So we calculate it by solving: total debt at depletion time = stream balance + 1. This ensures
// that we find the lowest timestamp at which the total debt exceeds the stream balance.
// Safe to use unchecked because the calculations cannot overflow or underflow.
unchecked {
- uint256 solvencyAmount = balanceScaled - snapshotDebtScaled + oneMVTScaled;
+ uint256 solvencyAmount = balanceScaled - snapshotDebtScaled - _ongoingDebtScaledOf(streamId) + oneMVTScaled;
uint256 solvencyPeriod = solvencyAmount / ratePerSecond;
// If the division is exact, return the depletion time.
if (solvencyAmount % ratePerSecond == 0) {
depletionTime = _streams[streamId].snapshotTime + solvencyPeriod;
}
// Otherwise, round up before returning since the division by rate per second has round down the result.
else {
depletionTime = _streams[streamId].snapshotTime + solvencyPeriod + 1;
}
}
}
Rerun the same test forge test --match-test test_audit_depletionTimeIssue --nmt testFork --via-ir -vvv
$ forge test --match-test test_audit_depletionTimeIssue --nmt testFork --via-ir -vvv
[⠊] Compiling...
[⠑] Compiling 4 files with Solc 0.8.26
[⠃] Solc 0.8.26 finished in 9.04s
Compiler run successful!
Ran 1 test for tests/integration/concrete/depletion-time-of/depletionTimeOf.t.sol:DepletionTimeOf_Integration_Concrete_Test
[PASS] test_audit_depletionTimeIssue() (gas: 193312)
Logs:
depletionTime: 1825148801
depletionTimeAfter100days: 1816508801
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.36ms (265.79µs CPU time)
The test passed after the suggested amendment, indicating that the calculated depletion time is now consistent with the
understanding that, with ongoing debts factored in, the stream fund will deplete earlier than previously expected.
