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

Precision loss in `FjordStaking::_checkEpochRollover` and `FjordStaking::calculateReward` causes tokens to be permanently locked in the `FjordStaking` contract and stakers not getting their expected amount of rewards

monstitude

Summary

The FjordStaking::calculateReward and FjordStaking::_checkEpochRollover calculates rewards for users based on their staked tokens. Both functions use PRECISION_18 to maintain precision during calculation. However, if the staked amount of tokens is larger than PRECISION_18, then precision is lost, leading to stakers not getting their expected amount of rewards and tokens to be permanently locked in the contract.

Vulnerability Details

Relevant Links

  1. https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L704

  2. https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L781

The FjordStaking::calculateReward and FjordStaking::_checkEpochRollover both use PRECISION_18 to maintain precision during the calculation of rewards for users based on their staked tokens. However if the total amount of staked tokens is much higher than PRECISION_18, it will cause a precision loss.

Impact

  • Stakers get slightly lower rewards due to precision loss

  • Small amount of tokens being permanently stuck inside the contract with no way of retrieving or claiming them and the amount of tokens stuck increasing slowly, possibly becoming a large value as more people stake overtime.

This causes a disruption in the functionality of calculating rewards for stakers.

Proof of Concept

The impact is demonstrated with the following test, which can be executed with forge test --mt testStakerRewardsAreDistributedCorrectly.

// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity ^0.8.21;
​
import "./FjordStakingBase.t.sol";
​
contract FjordPointsTest is FjordStakingBase {
function testStakerRewardsAreDistributedCorrectly() public {
address user1 = address(0x5);
address user2 = address(0x6);
address user3 = address(0x7);
address user4 = address(0x8);
​
deal(address(token), user1, 45_000 ether);
vm.startPrank(user1);
token.approve(address(fjordStaking), 45_000 ether);
fjordStaking.stake(45_000 ether);
vm.stopPrank();
​
deal(address(token), user2, 11 ether);
vm.startPrank(user2);
token.approve(address(fjordStaking), 11 ether);
fjordStaking.stake(11 ether);
vm.stopPrank();
​
deal(address(token), user3, 12_173 ether);
vm.startPrank(user3);
token.approve(address(fjordStaking), 12_173 ether);
fjordStaking.stake(12_173 ether);
vm.stopPrank();
​
deal(address(token), user4, 31_791 ether);
vm.startPrank(user4);
token.approve(address(fjordStaking), 31_791 ether);
fjordStaking.stake(31_791 ether);
vm.stopPrank();
​
_addRewardAndEpochRollover(10 ether, 6);
​
vm.prank(user1);
fjordStaking.claimReward(false);
​
vm.prank(user2);
fjordStaking.claimReward(false);
​
vm.prank(user3);
fjordStaking.claimReward(false);
​
vm.prank(user4);
fjordStaking.claimReward(false);
​
skip(4 weeks);
​
vm.startPrank(user1);
uint256 rewardAmount1 = fjordStaking.completeClaimRequest();
fjordStaking.unstakeAll();
vm.stopPrank();
​
vm.startPrank(user2);
uint256 rewardAmount2 = fjordStaking.completeClaimRequest();
fjordStaking.unstakeAll();
vm.stopPrank();
​
vm.startPrank(user3);
uint256 rewardAmount3 = fjordStaking.completeClaimRequest();
fjordStaking.unstakeAll();
vm.stopPrank();
​
vm.startPrank(user4);
uint256 rewardAmount4 = fjordStaking.completeClaimRequest();
fjordStaking.unstakeAll();
vm.stopPrank();
​
assertEq(rewardAmount1, 30345602697386906434); // decimal: 30.345602697386905
assertEq(rewardAmount2, 7417813992694577); // decimal: 0.007417813992694577
assertEq(rewardAmount3, 8208822703006462489); // decimal: 8.208822703006463
assertEq(rewardAmount4, 21438156785613936499); // decimal: 21.438156785613934
// If you add up all the values/decimals, it all adds up to 60e18 or 60
​
assert(token.balanceOf(address(fjordStaking)) > 10);
}
}

This test proves that stakers get slightly lower rewards that their expected amount and it also shows that tokens are being locked inside the contract.

Tools Used

Manual Review, Foundry

Recommendations

Increase the value of PRECISION_18 to 1e28 or higher to ensure that the new precision variable is always larger than any amount leading to accurate distribution. Also rename PRECISION_18 to reflect the updated value (e,g, PRECISION_28).

After adding this change, you can rerun the poc test provided earlier to verify that the rewards are being distributed correctly and stakers get their expected rewards.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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