Summary
Soulmate::getDivorced soulmates can continue to Airdrop::claim claim the LoveToken even if divorced. This is possible because theSoulmate::isDivorced function returns the msg.sender status.
Vulnerability Details
Soulmate::getDivorced soulmates can continue to Airdrop::claim the LoveToken even if divorced. This is possible because theSoulmate::isDivorced function returns the msg.sender status and not the status of a specific address passed as input.
When a user calls the Airdrop::claim function, the function checks if the user is divorced (calling soulmateContract.isDivorced() and reverting if the status is true). The isDivorced()function, as is written, returns the status of the msg.sender . In this case, the msg.sender is the Airdrop smart contract address (status = false) and not the user. The returned status is always false, the function doesn't revert and the divorced user can continue to claim their LoveToken.
Soulmate.sol
function isDivorced() public view returns (bool) {
@> return divorced[msg.sender];
}
Airdrop.sol
function claim() public {
@> if (soulmateContract.isDivorced()) revert Airdrop__CoupleIsDivorced();
....
Impact
function testCanClaimEvenIfDivorced() public isACouple {
vm.startPrank(soulmate1);
assertEq(loveToken.balanceOf(soulmate1), 0);
soulmateContract.getDivorced();
assertTrue(soulmateContract.isDivorced());
vm.stopPrank();
vm.startPrank(soulmate2);
assertEq(loveToken.balanceOf(soulmate2), 0);
assertTrue(soulmateContract.isDivorced());
vm.stopPrank();
vm.warp(block.timestamp + 200 days + 1 seconds);
vm.startPrank(soulmate1);
airdropContract.claim();
assertTrue(loveToken.balanceOf(soulmate1) == 200 ether);
vm.stopPrank();
vm.startPrank(soulmate2);
airdropContract.claim();
assertTrue(loveToken.balanceOf(soulmate2) == 200 ether);
vm.stopPrank();
}
Running 1 test for test/unit/SoulmateTest.t.sol:SoulmateTest
[PASS] testCanClaimEvenIfDivorced() (gas: 395363)
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.81ms
Tools Used
Manual review.
Recommendations
Pass the user address as an in input in the function.
ISolamate.sol
- function isDivorced() external view returns (bool);
+ function isDivorced(address user) external view returns (bool);
Soulmate.sol
- function isDivorced() public view returns (bool) {
+ function isDivorced(address user) public view returns (bool) {
- return divorced[msg.sender];
+ return divorced[user];
}
Airdrop.sol
function claim() public {
- if (soulmateContract.isDivorced()) revert Airdrop__CoupleIsDivorced();
+ if (soulmateContract.isDivorced(msg.sender)) revert Airdrop__CoupleIsDivorced();
....
Now if you run the test again, it correctly fails. The uses is divorced and can't claim the LoveToken.
function testCanClaimEvenIfDivorced() public isACouple {
vm.startPrank(soulmate1);
assertEq(loveToken.balanceOf(soulmate1), 0);
soulmateContract.getDivorced();
assertTrue(soulmateContract.isDivorced(soulmate1));
vm.stopPrank();
vm.startPrank(soulmate2);
assertEq(loveToken.balanceOf(soulmate2), 0);
assertTrue(soulmateContract.isDivorced(soulmate2));
vm.stopPrank();
vm.warp(block.timestamp + 200 days + 1 seconds);
vm.startPrank(soulmate1);
airdropContract.claim();
assertTrue(loveToken.balanceOf(soulmate1) == 200 ether);
vm.stopPrank();
vm.startPrank(soulmate2);
airdropContract.claim();
assertTrue(loveToken.balanceOf(soulmate2) == 200 ether);
vm.stopPrank();
}
Failing tests:
Encountered 1 failing test in test/unit/SoulmateTest.t.sol:SoulmateTest
[FAIL. Reason: Airdrop__CoupleIsDivorced()] testCanClaimEvenIfDivorced() (gas: 277782)
