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;
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
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;
}
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
*/
address user = makeAddr("user");
deal(address(token), user, 1 ether);
IFjordPoints(points).setStakingContract(address(fjordStaking));
vm.startPrank(user);
token.approve(address(fjordStaking), 1 ether);
uint256 stakedAmount = 1 ether;
fjordStaking.stake(stakedAmount);
assertEq(token.balanceOf(address(fjordStaking)), 1 ether);
uint256 startBalance = token.balanceOf(address(user));
uint256 unstakedAllAmount = fjordStaking.unstakeAll();
uint256 afterUnstakeAllBalance = token.balanceOf(address(user));
uint256 unstakeForEpochAmount = fjordStaking.unstake(1, stakedAmount);
uint256 afterUnstakeForEpochBalance = token.balanceOf(address(user));
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);
+ }
+ }