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

'UserData' does not reflects the values of 'DepositReceipt'

DanielEth

Vulnerability Details

When an user stakes, the value is correctly updated in 'FjordStaking::DepositReceipt', but it is not updated also in 'FjordStaking::UserData'.

In the last line of the function 'FjordStaking_redeem', we can see that 'ud.totalStaked' is calculated by adding 'deposit.staked' + 'deposit.vestedStaked'.

The problem here is that this happens at the start of the function and 'deposit.staked' is updated at the end of the function,
so when users stake, 'ud.totalStaked' will stuck at the old value while 'deposit.staked' is correclty updated.

Impact

If 'FjordStaking::userData' and 'FjordStaking::deposits' have different values, this could lead to data collision problems.

POC

function test_UserDataDoesNotReflectsDepositReceiptData(uint256 amount) public {
vm.startPrank(minter);
//Give 2000 Fjord to minter
deal(address(fjordToken), minter, 2000e18);
ERC20(fjordToken).approve(address(fjordStaking), 2000e18);
fjordStaking.addReward(2000e18);
vm.stopPrank();
​
address gianni = address(123);
vm.startPrank(gianni);
deal(address(fjordToken), gianni, 1_000e18); //assigns 1.000 FJORD to Gianni
uint256 startingGianniBalance = ERC20(fjordToken).balanceOf(address(gianni));
​
//Approve the staking contract to spend FJORD tokens
ERC20(fjordToken).approve(address(fjordStaking), type(uint256).max);
​
amount = bound(amount, 1, 1_000); // 1 - 1.000 FJORD
​
//stake
fjordStaking.stake(amount);
//Get data calling UserData
(uint256 totalStakedUserData,uint256 unclaimedRewards,uint16 unredeemedEpoch,uint16 lastClaimedEpoch) = fjordStaking.userData(address(gianni));
//Get data calling DepositReceipt
(uint16 epoch, uint256 totalStakedDepositReceipt, uint256 vestedStaked) = fjordStaking.deposits(gianni, (1));
assertNotEq(totalStakedUserData, totalStakedDepositReceipt); //As we can see, those values are differents.
}

Tools Used

Manual review, Foundry

Recommendations

Consider adding a logic that allows to updates 'FjordStaking::deposit' soneer that 'FjordStaking::userData', and remember to not update again 'FjordStaking::deposit' within the same transaction.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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