DeFiFoundry
20,000 USDC
View results
Submission Details
Severity: medium
Valid

onStreamCanceled() will always revert

Summary

onStreamCanceled() will always revert

Vulnerability Details

onStreamCanceled() call the _unstakedVested() to unstake a user from staking & points contract. When unstaking from points contract, _unstakedVested() uses msg.sender instead of steamOwner.

This is a issue because only sablier can call onStreamCanceled(), which means msg.sender in _unstakedVested() will be sablier, which means unstaking from points contract will be from sablier contract but there is no staking in points contract from sablier but was from streamOwner. therefore onStreamCanceled() will always revert

function onStreamCanceled(
uint256 streamId,
address sender,
uint128 senderAmount,
uint128 /*recipientAmount*/
) external override onlySablier checkEpochRollover {
address streamOwner = _streamIDOwners[streamId];
//SKIP//
-> _unstakeVested(streamOwner, streamId, amount);
emit SablierCanceled(streamOwner, streamId, sender, amount);
}
function _unstakeVested(address streamOwner, uint256 _streamID, uint256 amount) internal {
//SKIP//
-> points.onUnstaked(msg.sender, amount);
emit VestedUnstaked(streamOwner, epoch, amount, _streamID);
}

Impact

onStreamCanceled() will always revert due to using msg.sender instead of steamOwner in _unstakedVested()

Tools Used

VS code

Recommendations

In _unstakedVested()

- points.onUnstaked(msg.sender, amount);
+ points.onUnstaked(steamOwner, amount);
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Sablier calls `onStreamCanceled` > `_unstakeVested` > points.onUnstaked(msg.sender, amount); making the effects of points.onUnstaked apply to sablier who's the msg.sender.

Indeed the `points.onUnstaked` should use the streamOwner instead of msg.sender as an input parameter. Impact: high - The vested stakers who got their streams canceled will keep on receiving rewards (points included) for the previously staked stream. Likelihood: low - whenever a Sablier stream sender decides to `cancel()` a recipient's stream

Support

FAQs

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