DatingDapp

First Flight #33
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: high
Valid

`LikeRegistry::userBalances` mapping is not incremented, resulting in zero funds being sent to the created `MultiSigWallet` contract for every match

Description

When a user executes the LikeRegistry::likeUser function passing the address of the liked user, the value of the LikeRegistry::userBalances corresponding to the liked user is not incremented by 1 ether.

Impact

This results in zero funds being transferred to the created MultiSigWallet contract inside the LikeRegistry::matchRewards function if the like is mutual (i.e., the match is made).

Proof of Concept

  1. User (say Bob) executes the LikeRegistry::likeUser function, passing the address of the liked user (say Alice).

  2. The liked user (i.e., Alice) also executes the LikeRegistry::likeUser function passing the address of the user (i.e., Bob) who liked them.

  3. Now, since the like is mutual, when the liked user (i.e., Alice) executes the LikeRegistry::likeUser function, the likes[liked][msg.sender] value will be true, triggering the call to the LikeRegistry::matchRewards function.

  4. However, the value of userBalances[from] and userBalances[to] is still 0, because it was not incremented inside the LikeRegistry::likeUser function.

  5. So, when the rewards are being sent to the MultiSigWallet contract (corresponding to the match), their value is 0, resulting in zero funds being transferred.

Proof of Code

Since there isn't a way to track the MultiSigWallet contract address corresponding to the match, therefore I've added the return statements in the LikeRegistry::likeUser and LikeRegistry::matchRewards functions, basically a kind of mutation testing.

So, the revised LikeRegistry::likeUser and LikeRegistry::matchRewards functions would be:

function likeUser(address liked) external payable returns(address matchToMultiSigWallet) {
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");
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);
matchToMultiSigWallet = matchRewards(liked, msg.sender);
}
}
function matchRewards(address from, address to) internal returns(address) {
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;
// 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");
return address(multiSigWallet);
}

Then, create a test file named as DatingDappTest.t.sol inside the test folder with the following content:

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
import {Test} from "forge-std/Test.sol";
import {LikeRegistry} from "../src/LikeRegistry.sol";
import {SoulboundProfileNFT} from "../src/SoulboundProfileNFT.sol";
import {MultiSigWallet} from "../src/MultiSig.sol";
contract DatingDappTest is Test {
SoulboundProfileNFT soulboundNFT;
LikeRegistry likeRegistry;
address owner = makeAddr("owner");
address bob = makeAddr("bob");
address alice = makeAddr("alice");
function setUp() public {
vm.startPrank(owner);
soulboundNFT = new SoulboundProfileNFT();
likeRegistry = new LikeRegistry(address(soulboundNFT));
vm.stopPrank();
}
function testMultiSigIsNotGettingFundedOnMatch() public {
vm.prank(bob);
soulboundNFT.mintProfile("Bob", 25, "ipfs://profileImage");
vm.prank(alice);
soulboundNFT.mintProfile("Alice", 24, "ipfs://profileImage");
hoax(bob, 1 ether);
address multiSigForMatch = likeRegistry.likeUser{value: 1 ether}(alice);
assertEq(multiSigForMatch, address(0), "There is no match because it's only one-sided like till now");
hoax(alice, 1 ether);
multiSigForMatch = likeRegistry.likeUser{value: 1 ether}(bob);
assertNotEq(multiSigForMatch, address(0), "There is a match because it's a mutual like now");
// Logically, the MultiSigWallet contract balance should be 1.8 ether (1 + 1 - 0.2), but it's 0
assertEq(multiSigForMatch.balance, 0); // test ends here
}
}

Recommended Mitigation

To prevent this, the value of LikeRegistry::userBalances mapping should be incremented by 1 ether in the LikeRegistry::likeUser function.

+ uint256 constant REWARD_ON_LIKE = 1 ether;
function likeUser(address liked) external payable returns(address matchToMultiSigWallet) {
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");
likes[msg.sender][liked] = true;
+ userBalances[liked] += REWARD_ON_LIKE;
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);
matchToMultiSigWallet = matchRewards(liked, msg.sender);
}
}
Updates

Appeal created

n0kto Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_likeUser_no_userBalances_updated

Likelihood: High, always. Impact: High, loss of funds

Support

FAQs

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