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

`FjordStaking::unstakeAll` Does Not Unstake All User's Current Stakes

Summary

FjordStaking::unstakeAll does not unstake the user's stake for the current epoch and contradicts the function's nat spec here

Vulnerability Details

Function unstakeAll does does not work as intended.

PoC

Add the following functions to IFjordPoints.sol to make it a more complete interface.

pragma solidity =0.8.21;
interface IFjordPoints {
function onStaked(address user, uint256 amount) external;
function onUnstaked(address user, uint256 amount) external;
//added for audit
function claimPoints() external;
function setStakingContract(address) external;
function lastDistribution() external returns (uint256);
function pointsPerEpoch() external returns (uint256);
//-----------------------
}

Also, to prevent compiler errors add the following to the FjordPointsMock.sol

// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity =0.8.21;
import { IFjordPoints } from "src/interfaces/IFjordPoints.sol";
contract FjordPointsMock is IFjordPoints {
function onStaked(address user, uint256 amount) external pure {
user;
amount;
}
function onUnstaked(address user, uint256 amount) external pure {
user;
amount;
}
//Added audit
function claimPoints() external { }
function setStakingContract(address placeHolder) external { }
function lastDistribution() external returns (uint256) { }
function pointsPerEpoch() external returns (uint256) { }
//------------
}

Place the following test and imports into stake.t.sol

import { console } from "forge-std/Test.sol";
import { IFjordPoints } from "src/interfaces/IFjordPoints.sol";
function test_unstakeAllDoesNotWork() public {
/*
The unstakeAll function does not unstake for the current epoch.
To unstake for the current epoch users have to call the unstake with the current epoch as a parameter
*/
//SETUP:
//create a user that will stake in FjordStaking
address user = makeAddr("user");
//Dealing the user Fjord tokens so they can interact with the contract
deal(address(token), user, 1 ether);
// setting the staking contract in the points contract
IFjordPoints(points).setStakingContract(address(fjordStaking));
//START:
vm.startPrank(user);
// approving the staking contract
token.approve(address(fjordStaking), 1 ether);
uint256 stakedAmount = 1 ether;
fjordStaking.stake(stakedAmount);
//verifying the stake was successful
assertEq(token.balanceOf(address(fjordStaking)), 1 ether);
// Getting the balance of the user before they atemp to unstake their tokens
uint256 startBalance = token.balanceOf(address(user));
//Attempting to unstake with the unstakeAll function
uint256 unstakedAllAmount = fjordStaking.unstakeAll();
uint256 afterUnstakeAllBalance = token.balanceOf(address(user));
// Attempting to unstake for a specific epoch only
uint256 unstakeForEpochAmount = fjordStaking.unstake(1, stakedAmount);
uint256 afterUnstakeForEpochBalance = token.balanceOf(address(user));
//assert that the balance of the user unstaking for a specific epoch is greater than the balance after unstakeAll
assertGt(afterUnstakeForEpochBalance, afterUnstakeAllBalance);
console.log("The staked amount was: ", stakedAmount);
console.log("The balance after unstakedAll amount was: ", afterUnstakeAllBalance);
console.log("The balance after unstake for epoch was: ", afterUnstakeForEpochBalance);
}

Impact

FjordStaking::unstakeAll function does not work as intended according to the function name and nat spec. Thus, a user will not be unstaked from all epochs when calling this function.

Tools Used

Foundry and manual review

Recommendations

Make the following changes here

+ if (dr.epoch == currentEpoch) {
+ if (dr.staked > 0) {
+ unstake(dr.epoch, dr.staked);
+ }
+ }
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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