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

`stakeVested` function doesn't check if the stream has already been staked.

minato7namikazi

Vulnerability Details

function stakeVested(uint256 _streamID) external checkEpochRollover redeemPendingRewards {
//CHECK
if (!sablier.isStream(_streamID)) revert NotAStream();
if (sablier.isCold(_streamID)) revert NotAWarmStream();
​
// only allow authorized stream sender to stake cancelable stream
if (!authorizedSablierSenders[sablier.getSender(_streamID)]) {
revert StreamNotSupported();
}
if (address(sablier.getAsset(_streamID)) != address(fjordToken)) revert InvalidAsset();
​
uint128 depositedAmount = sablier.getDepositedAmount(_streamID);
uint128 withdrawnAmount = sablier.getWithdrawnAmount(_streamID);
uint128 refundedAmount = sablier.getRefundedAmount(_streamID);
​
if (depositedAmount - (withdrawnAmount + refundedAmount) <= 0) revert InvalidAmount();
​
uint256 _amount = depositedAmount - (withdrawnAmount + refundedAmount);
​
//EFFECT
userData[msg.sender].unredeemedEpoch = currentEpoch;
​
DepositReceipt storage dr = deposits[msg.sender][currentEpoch];
if (dr.epoch == 0) {
dr.vestedStaked = _amount;
dr.epoch = currentEpoch;
​
_activeDeposits[msg.sender].add(currentEpoch);
} else {
dr.vestedStaked += _amount;
}
​
_streamIDs[msg.sender][_streamID] = NFTData({ epoch: currentEpoch, amount: _amount });
_streamIDOwners[_streamID] = msg.sender;
newStaked += _amount;
newVestedStaked += _amount;
​
//INTERACT
sablier.transferFrom({ from: msg.sender, to: address(this), tokenId: _streamID });
points.onStaked(msg.sender, _amount);
​
emit VestedStaked(msg.sender, currentEpoch, _streamID, _amount);
}
​

in the stakeVested function in FjordStaking.sol,

The function doesn't check if the stream has already been staked. This could allow a user to stake the same stream multiple times, artificially inflating their stake and earning more rewards than they should.

To fix this, you should add a check to ensure the stream hasn't been staked before:

if (_streamIDs[msg.sender][_streamID].epoch != 0) revert StreamAlreadyStaked();

This check should be added near the beginning of the function, after the initial validations.

This bug could lead to an exploitable situation where a user repeatedly stakes the same stream, unfairly earning more rewards and potentially draining the contract of funds. It's crucial to ensure each unique stream can only be staked once per user because

  1. The function doesn't check if a stream has been previously staked.

  2. Each stake increases newStaked, newVestedStaked, and the user's deposit receipt.

  3. At epoch rollover, these values contribute to totalStaked and totalVestedStaked.

Impact

  1. User obtains a valid Sablier stream of 1000 FJO tokens.

  2. User calls stakeVested with this stream ID multiple times in the same epoch.

  3. Each call increases their stake without transferring additional tokens.

  4. At epoch rollover, the user's inflated stake is counted in totalStaked.

  5. The user now earns disproportionate rewards based on this inflated stake.

  6. This dilutes rewards for honest stakers and could drain the contract of rewards.

This exploit could significantly disrupt the fairness of the staking system, allowing malicious users to earn outsized rewards without additional risk or capital commitment. It undermines the entire reward distribution mechanism

Tools Used

Manual review

Updates

Lead Judging Commences

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

Support

FAQs

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