DeFiFoundry
20,000 USDC
View results
Submission Details
Severity: high
Invalid

Attacker can grieve users who have stakevested

Summary

In sablier, any "unkown caller" can call the "withdraw to recipient" function. An attacker can use this to cause loss of funds for the user.

Vulnerability Details

Once the nft is transferred to the contract thus making it the new recipient. The attacker can call the "withdraw to recipient" function, which will withdraw some portion of the funds to the recipient (which is the fjordstaking contract). (Access Control | Sablier Docs)

Thus while unstaking, the user gets back the stream, but will face a loss of funds equal to the amount withdrawn to the fjordStaking contract.

POC

  1. Alice calls stakeVested() (with 100FjordToken in the stream). The owner of the NFT/ recipient of the stream is now FjordStaking contract.

  2. Bob (an unknown caller) calls the "withdraw to recipient" function of the stream.

  3. This withdraws 20 FjordToken to the FjordStaking contract. The amount in stream is now 80FjordToken

  4. Alice unstakes and receives the NFT back. But she faces a loss of funds of 20tokens. (this is currently in the FjordStaking contract).

Note: This can also affect the reward calculation, since the balance of the contract increases, and this amount will be given out as rewards to other users.

Impact

Loss of funds for the user who has stakevested.

Tools Used

Manual Review

Recommendations

Have another function (hook called during withdrawal) to measure how much funds was withdrawn and refund them to that particular user while unstaking. Also, the amount being transferred while withdrawal happens, should not be given out as rewards. So the reward calculation also has to be updated as follows:

Note that the variable "lockedFjordToken" should be updated in the onStreamWithdrawn function(hook).

In the hook called during withdrawal, the stream owner and corresponding amount withdrawn must be stored.

uint256 pendingRewards = (currentBalance + totalVestedStaked + newVestedStaked)
- totalStaked - newStaked - totalRewards - lockedFjordToken; // updated calculation
Updates

Lead Judging Commences

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

Support

FAQs

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