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

Incorrect user passed to "onUnstaked" when stream is cancelled

PawelKedzia

Summary

Incorrect user passed to "onUnstaked" when a stream is canceled

Vulnerability Details

When a stream is canceled and the onStreamCanceled function is called, the streamOwner is passed to the _unstakeVested function. However, in the _unstakeVested function, when the points.onUnstaked https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L561 function is invoked, which is responsible for reducing the amount in the points contract, msg.sender is passed instead of streamOwner. In this case, msg.sender will be the Sablier contract, which will cause the system to attempt to subtract the amount not from the user's account but from the Sablier contract's account, which has a balance of zero. Subtracting the amount from zero will cause the entire transaction to revert. As a result, the user will continue accumulating rewards as if they are still staking, even though they no longer have the right to the refunded tokens.

To create a new test file that showcases the vulnerability within the FjordStaking.sol contract, we can focus on simulating the scenario described. Create a new test file inside the unit folder

// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity =0.8.21;
​
import "../FjordStakingBase.t.sol";
​
contract ExploitsTest is FjordStakingBase {
function beforeSetup() internal override {
// don't use mock contract to use real points contract
isMock = false;
}
​
function test_exploit_wrong_stream_owner() public {
// finish required set up
FjordPoints(points).setStakingContract(address(fjordStaking));
​
// alice is default reciepient inside createStreamAndStake
address reciepient = alice;
​
// address(this) is default sender
address sender = address(this);
​
uint256 stakedAmount = 10000000000000000000;
​
uint256 streamId =
createStreamAndStake({ cancelable: true, sender: sender, amount: stakedAmount });
​
(uint256 stakedAmountInFjordPointsBeforeCancel,,) = FjordPoints(points).users(reciepient);
​
assertEq(stakedAmount, stakedAmountInFjordPointsBeforeCancel);
​
ISablierV2LockupLinear(SABLIER_ADDRESS).cancel(streamId);
​
(uint256 stakedAmountInFjordPointsAfterCancel,,) = FjordPoints(points).users(reciepient);
​
// onStreamCanceled reverted, and the staked amount was not updated
assertEq(stakedAmount, stakedAmountInFjordPointsAfterCancel);
}
}
​

Impact

User acquires staking rewards after the stream is canceled, for the amount that was refunded to the stream sender.

Tools Used

Manual Review

Recommendations

In the _unstakeVested (https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L521) function change
in line https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L561
points.onUnstaked(msg.sender, amount);
to points.onUnstaked(streamOwner, 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.