DatingDapp

## Description `LikeRegistry::matchRewards` resets the matched users' `userBalances`. However, if a previously matched user gets liked and matched later on, the share multisig wallet will contain no funds from him. Technically, this scenario isn't possible due to a bug where `userBalances` is never updated with user funds, but the flawed logic is still there. ## Vulnerability Details Upon a match, `LikeRegistry::matchRewards` gets called. ```js function matchRewards(address from, address to) internal { uint256 matchUserOne = userBalances[from]; uint256 matchUserTwo = userBalances[to]; // [1] userBalances[from] = 0; userBalances[to] = 0; // [2] uint256 totalRewards = matchUserOne + matchUserTwo; // [3] 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); // [4] // Send ETH to the deployed multisig wallet (bool success,) = payable(address(multiSigWallet)).call{value: rewards}(""); require(success, "Transfer failed"); } ``` `matchRewards` will collect (`[2]`) all their previous like payments (minus a 10% fee `[3]`) and finally create a shared multisig wallet between the two users, which they can access for their first date (`[4]`). Note how `userBalances` is reset at `[1]`. Imagine the following scenario: 1. bob likes alice - `userBalance[bob] += 1 ETH` 2. bob likes angie - `userBalance[bob] += 1 ETH` 3. alice likes bob (match) - `userBalance[alice] += 1 ETH` - reset of `userBalance[alice]` and `userBalance[bob]` 4. angie likes alex - `userBalance[angie] += 1 ETH` 5. angie likes tony - `userBalance[angie] += 1 ETH` 6. angie likes bob (match) - `userBalance[angie] += 1 ETH` (total of 3 ETH) - `userBalance[bob]` is reset from bob's previous match with alice 7. shared multisig wallet is created using only angie's funds ## Proof of Concept To demonstrate the aforementioned scenario, apply the following patch in `LikeRegistry::likeUser` in order to update `userBalances` properly, ```diff function likeUser(address liked) external payable { require(msg.value >= 1 ether, "Must send at least 1 ETH"); require(!likes[msg.sender][liked], "Already liked"); require(msg.sender != liked, "Cannot like yourself"); require( profileNFT.profileToToken(msg.sender) != 0, "Must have a profile NFT" ); require( profileNFT.profileToToken(liked) != 0, "Liked user must have a profile NFT" ); + userBalances[msg.sender] += msg.value; likes[msg.sender][liked] = true; emit Liked(msg.sender, liked); // Check if mutual like if (likes[liked][msg.sender]) { matches[msg.sender].push(liked); matches[liked].push(msg.sender); emit Matched(msg.sender, liked); matchRewards(liked, msg.sender); } } ``` as well as a few logs in `LikeRegistry::matchRewards`: ```js function matchRewards(address from, address to) internal { uint256 matchUserOne = userBalances[from]; uint256 matchUserTwo = userBalances[to]; @> console.log("[LikeRegistry::matchRewards] matchUserOne:", matchUserOne); @> console.log("[LikeRegistry::matchRewards] matchUserTwo:", matchUserTwo); userBalances[from] = 0; userBalances[to] = 0; // ... // Deploy a MultiSig contract for the matched users MultiSigWallet multiSigWallet = new MultiSigWallet(payable(address(this)), from, to); // Send ETH to the deployed multisig wallet (bool success, ) = payable(address(multiSigWallet)).call{ value: rewards }(""); require(success, "Transfer failed"); @> console.log("[LikeRegistry::matchRewards] multiSigWallet balance:", address(multiSigWallet).balance); } ``` Finally, place `test_UserCanGetFreeDates` in `testSoulboundProfileNFT.t.sol`: ```js function test_UserCanGetFreeDates() public { address bob = makeAddr("bob"); address alice = makeAddr("alice"); address angie = makeAddr("angie"); address alex = makeAddr("alex"); address tony = makeAddr("tony"); vm.deal(bob, 10 ether); vm.deal(alice, 10 ether); vm.deal(angie, 10 ether); // mint a profile NFT for bob vm.prank(bob); soulboundNFT.mintProfile("Bob", 25, "ipfs://profileImage"); // mint a profile NFT for alice vm.prank(alice); soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage"); // mint a profile NFT for angie vm.prank(angie); soulboundNFT.mintProfile("Angie", 25, "ipfs://profileImage"); // mint a profile NFT for alex vm.prank(alex); soulboundNFT.mintProfile("Alex", 25, "ipfs://profileImage"); // mint a profile NFT for tony vm.prank(tony); soulboundNFT.mintProfile("Tony", 25, "ipfs://profileImage"); // bob <3 alice vm.prank(bob); likeRegistry.likeUser{value: 1 ether}(alice); assertTrue(likeRegistry.likes(bob, alice)); // bob <3 angie vm.prank(bob); likeRegistry.likeUser{value: 1 ether}(angie); assertTrue(likeRegistry.likes(bob, angie)); console.log("====== FIRST MATCH ======"); // alice <3 bob (match) vm.prank(alice); likeRegistry.likeUser{value: 1 ether}(bob); assertTrue(likeRegistry.likes(alice, bob)); // angie <3 alex vm.prank(angie); likeRegistry.likeUser{value: 1 ether}(alex); assertTrue(likeRegistry.likes(angie, alex)); // angie <3 tony vm.prank(angie); likeRegistry.likeUser{value: 1 ether}(tony); assertTrue(likeRegistry.likes(angie, tony)); console.log("\n\n====== SECOND MATCH ======"); // angie <3 bob (match) vm.prank(angie); likeRegistry.likeUser{value: 1 ether}(bob); } ``` and run the test: ```bash $ forge test --mt test_UserCanGetFreeDates -vvv Ran 1 test for test/testSoulboundProfileNFT.t.sol:SoulboundProfileNFTTest [PASS] test_UserCanGetFreeDates() (gas: 2274150) Logs: ====== FIRST MATCH ====== [LikeRegistry::matchRewards] matchUserOne: 2000000000000000000 [LikeRegistry::matchRewards] matchUserTwo: 1000000000000000000 [LikeRegistry::matchRewards] totalFees: 300000000000000000 [LikeRegistry::matchRewards] multiSigWallet balance: 2700000000000000000 ====== SECOND MATCH ====== [LikeRegistry::matchRewards] matchUserOne: 0 [LikeRegistry::matchRewards] matchUserTwo: 3000000000000000000 [LikeRegistry::matchRewards] totalFees: 600000000000000000 [LikeRegistry::matchRewards] multiSigWallet balance: 2700000000000000000 Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.57ms (1.93ms CPU time) Ran 1 test suite in 139.45ms (7.57ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests) ``` Note how on the second match (between angie and bob), `matchUserOne` (which corresponds to bob) is 0. It was reset upon his match with alice. ## Impact Users can get free dates. The impact is low since this bug isn't technically feasible due to a bug in the `LikeRegistry` contract where `userBalances` isn't properly updated with user payments. The logic flaw remains though. ## Recommendations Consider grouping ETH-like payments on a per-like basis instead of all together. ```diff contract LikeRegistry is Ownable { // ... mapping(address => mapping(address => bool)) public likes; mapping(address => address[]) public matches; - mapping(address => uint256) public userBalances; + mapping(address => mapping(address => uint256)) public userBalances; function likeUser(address liked) external payable { require(msg.value >= 1 ether, "Must send at least 1 ETH"); require(!likes[msg.sender][liked], "Already liked"); require(msg.sender != liked, "Cannot like yourself"); require( profileNFT.profileToToken(msg.sender) != 0, "Must have a profile NFT" ); require( profileNFT.profileToToken(liked) != 0, "Liked user must have a profile NFT" ); + userBalances[msg.sender][liked] += msg.value; likes[msg.sender][liked] = true; emit Liked(msg.sender, liked); // ... } function matchRewards(address from, address to) internal { - uint256 matchUserOne = userBalances[from]; - uint256 matchUserTwo = userBalances[to]; + uint256 matchUserOne = userBalances[from][to]; + uint256 matchUserTwo = userBalances[to][from]; - userBalances[from][to] = 0; - userBalances[to][from] = 0; + userBalances[from][to] = 0; + userBalances[to][from] = 0; // ... } } ```