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

People who don't love their soulmates anymore can claim `LoveToken` from `Airdrop::claim` function

bube

Summary

The Airdrop::claim function is intended to distribute LoveToken to participants who are in a couple. But due to an incorrect implementation on the Soulmate::isDivorced function, people who are no longer in a couple (divorced) can still claim LoveToken.

Vulnerability Details

The Airdrop::claim function allows the people who are soulmates to claim 1 LoveToken for every day in that they have been together. The README of the protocol said for the Soulmate::getDivorced function:
getDivorced: Where you and your soulmate are separated and no longer soulmates. This will cancel the possibily for 2 lovers to collect LoveToken from the airdrop.

Therefore, people who aren't anymore soulmates should not have the possibility to claim LoveToken from Airdrop.

​
function claim() public {
// No LoveToken for people who don't love their soulmates anymore.
@> if (soulmateContract.isDivorced()) revert Airdrop__CoupleIsDivorced();
​
// Calculating since how long soulmates are reunited
uint256 numberOfDaysInCouple = (block.timestamp -
soulmateContract.idToCreationTimestamp(
soulmateContract.ownerToId(msg.sender)
)) / daysInSecond;
.
.
.
}

The Airdrop::claim function includes a check intended to prevent divorced participants from claiming LoveTokens:
if (soulmateContract.isDivorced()) revert Airdrop__CoupleIsDivorced();

The Soulmate::isDivorced function has the following implementation:

​
function isDivorced() public view returns (bool) {
return divorced[msg.sender];
}

The isDivorced() function doesn't accept any input arguments. It checks if the caller of the function (msg.sender) is divorced. But when the Airdrop::claim function calls the Soulmate::isDivorced() function, the caller is the Airdrop contract, not the caller of the claim function (soulmate1 or soulmate2). The address of Airdrop contract is not in the divorced mapping. Therefore, the return value from the call to the Soulmate::isDivorced will be always false.

Impact

The Airdrop::claim function allows any divorced couple to claim 1 LoveToken per a day.
The following test function test_ClaimDivorcedCouple() shows that case. In this test scenario we have a soulmate1 and soulmate2. They are firstly a couple, but then they decided to get divorced. Although that, the claim function continues to allow them to claim LoveToken.
You can add this function in the file AirdropTest.t.sol and execute it with the Foundry command: forge test --match-test "test_ClaimDivorcedCouple"

function test_ClaimDivorcedCouple() public {
//soulmate1 and soulmate2 are couple
_mintOneTokenForBothSoulmates();
vm.prank(soulmate1);
//soulmate1 and soulmate2 get divorced
soulmateContract.getDivorced();
​
vm.warp(block.timestamp + 5 days);
​
vm.prank(soulmate1);
//claim function should revert, soulmate1 is divorced
vm.expectRevert();
airdropContract.claim();
​
vm.prank(soulmate2);
//claim function should revert, soulmate2 is divorced
vm.expectRevert();
airdropContract.claim();
}

The test doesn't revert as expected:

Failing tests:
Encountered 1 failing test in test/unit/AirdropTest.t.sol:AirdropTest
[FAIL. Reason: call did not revert as expected] test_ClaimDivorcedCouple() (gas: 330782)

The divorced couple (soulmate1 and soulmate2) successfully claim 5 LoveToken from the Airdrop which is not the intended logic.

Tools Used

VS Code, Foundry

Recommendations

In order to mitigate this issue you can add an input argument address to the Soulmate::isDivorced function:

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

Then you should modify the Airdrop::claim function to calls the Soulmate::isDivorced function in a proper way:

function claim() public {
// No LoveToken for people who don't love their soulmates anymore.
+ if (soulmateContract.isDivorced(msg.sender)) revert Airdrop__CoupleIsDivorced();
- if (soulmateContract.isDivorced()) revert Airdrop__CoupleIsDivorced();
​
// Calculating since how long soulmates are reunited
uint256 numberOfDaysInCouple = (block.timestamp -
soulmateContract.idToCreationTimestamp(
soulmateContract.ownerToId(msg.sender)
)) / daysInSecond;
.
.
.
}

After these changes the test function test_ClaimDivorcedCouple reverted as expected and the divorced couple can not claim a LoveToken. The changes in the interface of the Soulmate contract also should be made. Also, you should decide if the isDivorced function should be called by anyone.

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.