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

Logic error in Soulmate::isDivorced() allowings divorced Soulmates to claim daily airdrops

dougo

Summary

According to protocol documentation, divorced Soulmates should not be able to continue to claim daily loveToken airdrops once they have been divorced. Due to a logic error in the Soulmate::isDivorced() function, it will always return false when called in the Airdrop::claim() function. This allows the divorced Soulmates to continue claiming airdropped loveTokens.

Vulnerability Details

When the Airdrop::claim() function calls Soulmate::isDivorced(), the msg.caller is used to access the divorced mapping. The value of msg.caller within Soulmate::isDivorced() is the Airdrop contract and is NOT the divorced Soulmate who called Airdrop::claim(). The Airdrop contract doesn't exist in the divorced mapping, so false will be returned by Soulmate::isDivorced().

The Foundry test below demonstrates the problem as it shows that an airdrop claim from a divorced Soulmate will NOT revert as expected and the test will fail:

function testAudit_DivorcedAirdrop() public {
_mintOneTokenForBothSoulmates();
​
vm.warp(block.timestamp + 1 days + 1 seconds);
​
vm.startPrank(soulmate1);
soulmateContract.getDivorced();
vm.expectRevert();
airdropContract.claim();
vm.stopPrank();
}

Impact

High

Tools Used

Visual Studio Code, Foundry

Recommendations

It is recommended to rewrite Soulmate::isDivorced() to take a parameter which accepts an address which is to be checked for being divorced.

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

As a result, Airdrop::claim() should also be modified as shown to pass the parameter - the caller of Airdrop::claim():

function claim() public {
// No LoveToken for people who don't love their soulmates anymore.
- if (soulmateContract.isDivorced()) revert Airdrop__CoupleIsDivorced();
+ if (soulmateContract.isDivorced(msg.sender)) revert Airdrop__CoupleIsDivorced();
// remainder of function
}

Finally, the ISoulmate contract should be changed to reflect the new signature of Soulmate::isDivorced():

- function isDivorced() external view returns (bool);
+ function isDivorced(address) external view returns (bool);

After these changes, the test provided in the Vulnerability Details section should pass.

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.