DatingDapp

First Flight #33
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: low
Invalid

Inability to Match Mutual Users Again in the Future.

Summary

The LikeRegistry contract has a design flaw where the likes mapping is not reset after a mutual match. This prevents users from matching again in the future, as the likes mapping remains true indefinitely. This issue limits the functionality of the contract and prevents users from re-engaging with each other after an initial match.

Vulnerability Details

When a user decides to like other user, he calls LikeRegistry::likeUser which then calls LikeRegistry::matchRewards if there are matches. The likes mapping is used to track whether a user has liked another user. Once a mutual like occurs, the users are matched, and their addresses are added to the matches mapping. However, the likes mapping is not reset after a match, meaning that users cannot like each other again in the future. This is because the likeUser function includes a check to ensure that a user cannot like another user more than once (require(!likes[msg.sender][liked], "Already liked");). But after MultiSigWallet has been created for both users and their funds sent to it, their likes mappings should be reset so they can be able to re-engage in the future.

Code Snippet

function likeUser(address liked) external payable {
...
@> require(!likes[msg.sender][liked], "Already liked");
...
// Check if mutual like
if (likes[liked][msg.sender]) {
...
matchRewards(liked, msg.sender);
}
}
function matchRewards(address from, address to) internal {
uint256 matchUserOne = userBalances[from]; //@audit-issue it will always be zero, not added in likeUser() -ref: A1
uint256 matchUserTwo = userBalances[to];
userBalances[from] = 0;
userBalances[to] = 0;
uint256 totalRewards = matchUserOne + matchUserTwo;
uint256 matchingFees = (totalRewards * FIXEDFEE) / 100;
uint256 rewards = totalRewards - matchingFees;
totalFees += matchingFees;
// Deploy a MultiSig contract for the matched users
MultiSigWallet multiSigWallet = new MultiSigWallet(from, to); //@audit-issue No tracking of multisig wallet. No event emitted or state variable updated
// Send ETH to the deployed multisig wallet
(bool success,) = payable(address(multiSigWallet)).call{value: rewards}("");
require(success, "Transfer failed");
}

Proof of Concept

The provided test case test_UnableToMatchMutualsAgain demonstrates the issue. After USER1 and USER2 mutually like each other and MultiSigWallet created for them, USER1 is unable to like USER2 again in the future because the likes mapping is not reset.

Code

Here is the code the prove the scenario. Create a new test file LikeRegistryTest.t.sol and add to the test/ directory.

Then run the command below;

forge test --mt test_UnableToMatchMutualsAgain
import {LikeRegistry} from "../src/LikeRegistry.sol";
import {SoulboundProfileNFT} from "../src/SoulboundProfileNFT.sol";
import {console, Test} from "forge-std/Test.sol";
contract LikeRegistryTest is Test {
SoulboundProfileNFT soulboundNFT;
LikeRegistry likeRegistry;
address user1 = makeAddr("USER1");
address user2 = makeAddr("USER2");
uint256 constant STARTING_BALANCE = 100 ether;
uint256 constant DEPOSIT_AMOUNT = 2 ether;
function setUp() public {
soulboundNFT = new SoulboundProfileNFT();
likeRegistry = new LikeRegistry(address(soulboundNFT));
vm.deal(user1, STARTING_BALANCE);
vm.deal(user2, STARTING_BALANCE);
}
//Modifier to handle minting profile for both users
modifier mintBothUsers() {
vm.prank(user1);
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
vm.prank(user2);
soulboundNFT.mintProfile("Bob", 29, "ipfs://BobprofileImage");
uint256 user1TokenId = soulboundNFT.profileToToken(user1);
assertEq(user1TokenId, 1, "Token ID for user 1 should be 1");
uint256 user2TokenId = soulboundNFT.profileToToken(user2);
assertEq(user2TokenId, 2, "Token ID for user 2 should be 2");
_;
}
function test_UnableToMatchMutualsAgain() public mintBothUsers {
//USER 1 liked a user and deposited 2 ether
vm.prank(user1);
likeRegistry.likeUser{value: DEPOSIT_AMOUNT}(user2);
vm.prank(user2);
likeRegistry.likeUser{value: DEPOSIT_AMOUNT}(user1);
assertEq(likeRegistry.matches(user1, 0), user2);
assertEq(likeRegistry.matches(user2, 0), user1);
//User 1 tries to like user 2 in the future
vm.warp(block.timestamp + 5555);
vm.prank(user1);
vm.expectRevert("Already liked");
likeRegistry.likeUser{value: DEPOSIT_AMOUNT}(user2);
}
}

Impact

The impact of this issue is as follows:

  • Users are unable to like each other again after an initial mutual match, limiting the functionality of the contract.

  • This design flaw could lead to a poor user experience, as users expect to be able to interact with each other multiple times.

Tools Used

  • Manual review

  • Foundry (Forge) for testing and vulnerability demonstration.

Recommendations

To address this issue, the likes mapping should be reset after a mutual match. This can be achieved by updating the matchRewards function to reset the likes mapping for both users involved in the match. Here is the recommended fix:

function matchRewards(address from, address to) internal {
uint256 matchUserOne = userBalances[from];
uint256 matchUserTwo = userBalances[to];
userBalances[from] = 0;
userBalances[to] = 0;
uint256 totalRewards = matchUserOne + matchUserTwo;
uint256 matchingFees = (totalRewards * FIXEDFEE) / 100;
uint256 rewards = totalRewards - matchingFees;
totalFees += matchingFees;
// Reset likes mapping for both users
+ likes[from][to] = false;
+ likes[to][from] = false;
// Deploy a MultiSig contract for the matched users
MultiSigWallet multiSigWallet = new MultiSigWallet(from, to);
// Send ETH to the deployed multisig wallet
(bool success,) = payable(address(multiSigWallet)).call{value: rewards}("");
require(success, "Transfer failed");
}

By resetting the likes mapping, users will be able to like each other again in the future, enabling repeated interactions and improving the overall user experience.

Updates

Appeal created

n0kto Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelyhood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.