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

Any non-minter can claim staking rewards by incorrectly identifying himself as the owner of the NFT at index 0

0xlrivo

Summary

Staking::claimRewards() allows non-minters to fraudulently claim airdrop rewards by falsely identifying themselves as the owner of the NFT at index 0.

Vulnerability Details

This vulnerability is made possible because:

  • Soulmate::ownerToId() incorrectly returns 0 for both the actual owners of the corresponding NFT's index and for individuals who have not yet minted an NFT

  • the staking contract incorrectly initialize the lastClaim mapping

If a non-minter tries to call Staking::claimRewards() his staking time will be initialised at the minting time of the NFT at index 0

if (lastClaim[msg.sender] == 0) {
lastClaim[msg.sender] = soulmateContract.idToCreationTimestamp(soulmateId);
}

Also, due to the second point, anyone can obtain more rewards (even legitimate users), by depositing more tokens just before claiming staking rewards.

Impact

The vulnerability enables unauthorised individuals to illegitimately accumulate rewards over time, thereby increasing the daily number of claimable tokens.

Tools Used

Add the following code to StakingTest.t.sol:

function test_CanClaimStakingRewardsByDepositingInSameTxn() public {
loveToken.testMint(attacker, 100e18);
uint256 initialBalance = loveToken.balanceOf(attacker);
vm.startPrank(attacker);
loveToken.approve(address(stakingContract), initialBalance);
stakingContract.deposit(initialBalance); // deposit all in staking contract
stakingContract.claimRewards(); // and claim all rewards
stakingContract.withdraw(initialBalance); // and withdraw the deposited funds
vm.stopPrank();
uint256 finalBalance = loveToken.balanceOf(attacker);
console2.log("Balance after claiming staking: ", finalBalance);
}

PS: I added a testMint() function to the LoveToken contract just for testing purposes

Recommendations

  1. make Soulmate::nextId start from 1 instead of 0

  2. rename the mapping Soulmate::ownerToId to Soulmate::_ownerToId and make it private

  3. add a wrapper public function to the now private mapping Soulmate::_ownerToId that throws if the returned id is 0

mapping(address owner => uint256 id) private _ownerToId;
​
function ownerToId(address _owner) public view returns (uint256) {
uint256 id = _ownerToId[_owner];
if (id != 0) return id;
else revert Soulmate_invalidId();
}

By renaming you don't have to change all the other contracts that requires Soulmate::ownerToId() to work.

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.