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

It's possible to abuse the streaming property of Sablier streams to game the staking system

y4y

Summary

The FjordStaking contract allows users to stake Fjord tokens and streams from Sablier protocol to accrue Fjord points. The logic in handling Sablier streams is faulty, which allows a premitted Sablier staker to game the system to get more points.

Vulnerability Details

The core logic behind Sablier's streaming service is to distribute a given amount of tokens equally across set duration, for example, a stream may send total 1000 tokens in the course of 10 days, this means the stream would send 100 tokens per day, to eventually accumulate 1000 tokens at the end of stream. There are also varities of options on how users can send their funds, some sends a fixed amount, some will lock the funds for an amount of time, and then dirtsibute the rest funds. The reason of introducing the concept of streams is due to how it's handled in stakeVested:

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);
}

We see that staked amount is determined from depositedAmount - (withdrawnAmount + refundedAmount);, for the sake of simplicity, let's assume withdrawn and refunded amounts are both none, which then leads to amount = depositedAmount. This will be the staked amount recorded in the contract, and user's reward is also calculated based on it. And from one of the contract in Sablier's repository, we see:

// SablierV2Lockup.sol
function getDepositedAmount(
uint256 streamId
)
external
view
override
notNull(streamId)
returns (uint128 depositedAmount)
{
depositedAmount = _streams[streamId].amounts.deposited;
}
​

And such variable is set in:

// SablierV2LockupLinear.sol
​
_streams[streamId] = Lockup.Stream({
amounts: Lockup.Amounts({ deposited: createAmounts.deposit, refunded: 0, withdrawn: 0 }),
asset: params.asset,
endTime: params.timestamps.end,
isCancelable: params.cancelable,
isDepleted: false,
isStream: true,
isTransferable: params.transferable,
sender: params.sender,
startTime: params.timestamps.start,
wasCanceled: false
});
​
// Effect: set the cliff time if it is greater than zero.
if (params.timestamps.cliff > 0) {
_cliffs[streamId] = params.timestamps.cliff;
}
​
// Effect: bump the next stream ID.
// Using unchecked arithmetic because these calculations cannot realistically overflow, ever.
unchecked {
nextStreamId = streamId + 1;
}
​
// Effect: mint the NFT to the recipient.
_mint({ to: params.recipient, tokenId: streamId });
​
// Interaction: transfer the deposit amount.
params.asset.safeTransferFrom({ from: msg.sender, to: address(this), value: createAmounts.deposit });
​

So, for a stream which will total send 1000 tokens, user will deposit this entire amount clean, then the stream can be created, and this makes the deposited amount to be 1000 tokens, as expected. However, remember that a duration can be set for streams, and for a linear stream, funds will be sent equally through out the duration.

Note that each epoch set for staking round is 7 days, and it's possible to set the Sablier duration to a much longer time span, for example, a year. In this case, suppose Alice deposits 365000 tokens as stream, sets the duration to one year, which makes the daily flow to be 1000 tokens. Immediately after, Alice stakes this newly created stream, with amount being 365000. After 35 days, which means 5 whole epoch rounds has passed, Alice has accrued enough points, and wants to withdraw/unstake her stream. At this point, Alice has transferred 35000 tokens to FjordStaking, which is less than 1/10 of the total amont she is supposed to put in, but she will get rewards as if she actually staked 365000 tokens.

Impact

As explained above, a malicious user can abuse the streaming property and game the system, to pay more tokens, but get more points.

Tools Used

Manual review

Recommendations

Calculate and distribute rewards for streams based on how much the contract actually received, instead of using deposited amount directly.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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