Summary
The SablierFlow::pause function in the Sablier smart contract updates the snapshot time to the current block timestamp. This behavior contradicts the NatSpec documentation in the ISablierFlow interface, which specifies that the snapshot time should not be modified when pausing the stream.
Vulnerability Details
The SablierFlow::pause function calls an internal function, SablierFlow::_pause, which then invokes SablierFlow::_adjustRatePerSecond. This _adjustRatePerSecond function updates the snapshot time to the current block timestamp, causing a mismatch with the behavior described in the NatSpec. The relevant code segments are shown below:
function pause(uint256 streamId)
external
override
noDelegateCall
notNull(streamId)
notPaused(streamId)
onlySender(streamId)
updateMetadata(streamId)
{
_pause(streamId);
}
function _pause(uint256 streamId) internal {
_adjustRatePerSecond({ streamId: streamId, newRatePerSecond: ud21x18(0) });
emit ISablierFlow.PauseFlowStream({
streamId: streamId,
sender: _streams[streamId].sender,
recipient: _ownerOf(streamId),
totalDebt: _totalDebtOf(streamId)
});
}
function _adjustRatePerSecond(uint256 streamId, UD21x18 newRatePerSecond) internal {
if (newRatePerSecond.unwrap() == _streams[streamId].ratePerSecond.unwrap()) {
revert Errors.SablierFlow_RatePerSecondNotDifferent(streamId, newRatePerSecond);
}
uint256 ongoingDebtScaled = _ongoingDebtScaledOf(streamId);
if (ongoingDebtScaled > 0) {
_streams[streamId].snapshotDebtScaled += ongoingDebtScaled;
}
@> _streams[streamId].snapshotTime = uint40(block.timestamp);
_streams[streamId].ratePerSecond = newRatePerSecond;
}
Impact
Updating the snapshot time during a pause operation may lead to unexpected behavior, diverging from the documented expectations in the NatSpec. This discrepancy could cause issues in downstream operations or lead to confusion regarding the intended behavior of pause.
Tools Used
Manual review.
Recommendations
Since SablierFlow::_adjustRatePerSecond is used by other functions, we recommend creating an alternative version of this function that omits the snapshot time update for cases where it is not needed.
/// @dev See the documentation for the user-facing functions that call this internal function.
function _pause(uint256 streamId) internal {
_adjustRatePerSecondForPause({ streamId: streamId, newRatePerSecond: ud21x18(0) });
// Log the pause.
emit ISablierFlow.PauseFlowStream({
streamId: streamId,
sender: _streams[streamId].sender,
recipient: _ownerOf(streamId),
totalDebt: _totalDebtOf(streamId)
});
}
/// @dev See the documentation for the user-facing functions that call this internal function.
function _adjustRatePerSecondForPause(uint256 streamId, UD21x18 newRatePerSecond) internal {
// Check: the new rate per second is different from the current rate per second.
if (newRatePerSecond.unwrap() == _streams[streamId].ratePerSecond.unwrap()) {
revert Errors.SablierFlow_RatePerSecondNotDifferent(streamId, newRatePerSecond);
}
uint256 ongoingDebtScaled = _ongoingDebtScaledOf(streamId);
// Update the snapshot debt only if the stream has ongoing debt.
if (ongoingDebtScaled > 0) {
// Effect: update the snapshot debt.
_streams[streamId].snapshotDebtScaled += ongoingDebtScaled;
}
- // Effect: update the snapshot time.
- _streams[streamId].snapshotTime = uint40(block.timestamp);
// Effect: set the new rate per second.
_streams[streamId].ratePerSecond = newRatePerSecond;
}
