First Flight #9: Soulmate
First Flight #9
Beginner FriendlyFoundryNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

The Staking::claimRewards function fails to store the staking amount along with its timestamp, conducting all calculations based on the previous call of claimRewards

naman1729

Summary

  • This function fails to store the staking amount along with its timestamp, conducting all calculations based on the previous call of claimRewards rather than considering the timestamp associated with the staking amount.

Vulnerability Details

  • This function fails to store the staking amount along with its timestamp, conducting all calculations based on the previous call of claimRewards rather than considering the timestamp associated with the staking amount.

/// @notice Claim rewards for staking.
/// @notice Users can claim 1 token per staking token per week.
function claimRewards() public {
uint256 soulmateId = soulmateContract.ownerToId(msg.sender);
// first claim
if (lastClaim[msg.sender] == 0) {
lastClaim[msg.sender] = soulmateContract.idToCreationTimestamp(
soulmateId
);
}
​
// How many weeks passed since the last claim.
// Thanks to round-down division, it will be the lower amount possible until a week has completly pass.
uint256 timeInWeeksSinceLastClaim = ((block.timestamp -
lastClaim[msg.sender]) / 1 weeks);
​
if (timeInWeeksSinceLastClaim < 1)
revert Staking__StakingPeriodTooShort();
​
lastClaim[msg.sender] = block.timestamp;
​
// Send the same amount of LoveToken as the week waited times the number of token staked
uint256 amountToClaim = userStakes[msg.sender] *
timeInWeeksSinceLastClaim;
loveToken.transferFrom(
address(stakingVault),
msg.sender,
amountToClaim
);
​
emit RewardsClaimed(msg.sender, amountToClaim);
}

POC

  • On testing the Staking::claimRewards function, which is designed to manage the claiming of staking rewards. The PoC reveals an issue with the tracking of staking amounts and their associated timestamps within the current implementation. Specifically, the function fails to correctly handle scenarios where the staking amount timestamp is nearing one week and additional staking occurs before this threshold. As a result, the calculation includes all previously staked amounts along with the recent stake amount, which introduces an unfairness in the staking process.

function test_ClaimRewardsWhenMajorityDepositDoneAtLastOfWeek() public {
uint balancePerSoulmates = 5 ether;
uint weekOfStaking = 5;
uint extraStaking = 10 ether;
​
_giveLoveTokenToSoulmates(balancePerSoulmates);
​
vm.startPrank(soulmate1);
loveToken.approve(address(stakingContract), balancePerSoulmates - 4 ether);
stakingContract.deposit(balancePerSoulmates - 4 ether);
vm.stopPrank();
​
vm.warp(block.timestamp + (weekOfStaking - 1) * 1 weeks + 6 days);
// After 4 weeks 6 days.
​
vm.startPrank(soulmate1);
loveToken.approve(address(stakingContract), 4 ether);
stakingContract.deposit(4 ether);
vm.stopPrank();
​
vm.warp(block.timestamp + 1 days + 1 seconds);
// After 5 weeks
​
vm.prank(soulmate1);
stakingContract.claimRewards();
​
assertTrue(
loveToken.balanceOf(soulmate1) ==
weekOfStaking * balancePerSoulmates
);
}

Impact

  • keeping no track of the timestamp for all stanking amount

  • which cause the over rewarding to the user.

Tools Used

  • Manual Review

Recommendations

  • we can introduce mapping which keep track of the amount deposit with the block.timestamp.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-claimRewards-multi-deposits-time

High severity, this allows users to claim additional rewards without committing to intended weekly staking period via multi-deposit/deposit right before claiming rewards.

Support

FAQs

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