Fjord Token Staking
Fjord
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

onStreamCanceled function breaks points accounting

0xCiphky

Vulnerability Details:

The onStreamCanceled function is called from the Sablier contract to unstake a stream owners vested FJORD tokens. This function invokes _redeem to first update the streamOwner’s rewards, followed by _unstakeVested to partially or fully unstake the vested FJORD tokens.

function _unstakeVested(address streamOwner, uint256 _streamID, uint256 amount) internal {
NFTData storage data = _streamIDs[streamOwner][_streamID];
DepositReceipt storage dr = deposits[streamOwner][data.epoch];
if (amount > data.amount) revert InvalidAmount();
...
​
points.onUnstaked(msg.sender, amount);
​
emit VestedUnstaked(streamOwner, epoch, amount, _streamID);
}

The problem is that the onStreamCanceled function has the onlySablier modifier, meaning msg.sender will always be the Sablier address. However, in the _unstakeVested function, msg.sender is used to update the points by calling points.onUnstaked, as shown above.

function onUnstaked(address user, uint256 amount)
external
onlyStaking
checkDistribution
updatePendingPoints(user)
{
UserInfo storage userInfo = users[user];
if (amount > userInfo.stakedAmount) {
revert UnstakingAmountExceedsStakedAmount();
}
userInfo.stakedAmount = userInfo.stakedAmount.sub(amount);
totalStaked = totalStaked.sub(amount);
emit Unstaked(user, amount);
}

As a result, the onUnstaked function will mistakenly assign the Sablier address as the user variable. This leads to a function revert since the initial points when staked were correctly attributed to the streamOwner.

Since the onStreamCanceled function is invoked in a try-catch block, this revert causes the points.onUnstaked call to be skipped. Consequently, the user's totalStaked in the FjordPoints contract remains unchanged, allowing them to continue earning points on the unstaked amount, thus receiving more rewards than they should.

Impact:

  • Severity: Medium. Users will gain more rewards than they should, and this can also be used to exploit the system to earn extra rewards.

  • Likelihood: High. This will occur every time the onStreamCanceled function is called.

Tools Used:

  • Manual analysis

Recommendation:

To resolve this issue, ensure that the streamOwner is passed to the points.onUnstaked function instead of msg.sender. This will correctly attribute the unstaking to the streamOwner and adjust their points accordingly.

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.