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

The deployed `MultiSigWallet` address cannot be fetched when a match is created, making it impossible for users / owners to access or transfer funds

syedasadkazmii

Description

When there is a mutual like, the LikeRegistry::matchRewards function is supposed to be called by the LikeRegistry::likeUser function, which is deploying a MultiSigWallet contract for the matched users. But, the address of the deployed MultiSigWallet contract is not being returned, emitted or stored anywhere, making it impossible for the underlying owners (i.e., matched users) of the MultiSigWallet contract to access / transfer the funds available in that contract.

Impact

The matched users wouldn't be able to access the funds available inside their MultiSigWallet contract.

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. Inside the LikeRegistry::matchRewards function, the MultiSigWallet contract is deployed and the rewards are transferred to the MultiSigWallet contract.

  5. The address of the deployed MultiSigWallet contract remains unknown because it's nether returned, emitted or stored anywhere.

  6. The owners of the deployed MultiSigWallet contract are the underlying matched users. However, they can't access the funds inside the MultiSigWallet contract because there is no way to interact with it without knowing its address.

Proof of Code

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 testUnableToFetchMultiSigWalletAddress() public {
vm.prank(bob);
soulboundNFT.mintProfile("Bob", 25, "ipfs://profileImage");
​
vm.prank(alice);
soulboundNFT.mintProfile("Alice", 24, "ipfs://profileImage");
​
hoax(bob, 1 ether);
likeRegistry.likeUser{value: 1 ether}(alice);
​
hoax(alice, 1 ether);
likeRegistry.likeUser{value: 1 ether}(bob);
​
// There is a match between Bob and Alice because it's a mutual like, but neither Bob nor Alice can access the funds inside their MultiSigWallet contract because they don't know its address
}
}

Recommended Mitigation

The easiest recommendation would be to return the address of the deployed MultiSigWallet contract by adding the return statement at the end of the LikeRegistry::matchRewards function followed by assigning the same inside the mutual like condition of the LikeRegistry::likeUser function.

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);
}

Now, you can get the address of the deployed MultiSigWallet contract in the above DatingDappTest as:

function testFetchMultiSigWalletAddress() 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);
​
// There is a match between Bob and Alice because it's a mutual like
assertNotEq(multiSigForMatch, address(0), "There is a match because it's a mutual like now");
}

Or, you could also create a mapping to store the MultiSigWallet contract address corresponding to the combined hash of the addresses of the matched users, like:

mapping(bytes32 matchHash => address multiSigWallet) public matchToMultiSigWallet;

And, add the following line inside the LikeRegistry::matchRewards function:

// key => hash of the matched users' addresses => keccak256(abi.encode(from, to))
// value => address of the `MultiSigWallet` contract address => address(multiSigWallet)
​
matchToMultiSigWallet[keccak256(abi.encode(from, to))] = address(multiSigWallet);

So, the above testFetchMultiSigWalletAddress test function can be modified as:

function testFetchMultiSigWalletAddress() public {
vm.prank(bob);
soulboundNFT.mintProfile("Bob", 25, "ipfs://profileImage");
​
vm.prank(alice);
soulboundNFT.mintProfile("Alice", 24, "ipfs://profileImage");
​
hoax(bob, 1 ether);
likeRegistry.likeUser{value: 1 ether}(alice);
​
hoax(alice, 1 ether);
likeRegistry.likeUser{value: 1 ether}(bob);
​
address multiSigWalletAddress = likeRegistry.matchToMultiSigWallet(keccak256(abi.encode(bob, alice)));
​
// There is a match between Bob and Alice because it's a mutual like
assertNotEq(multiSigWalletAddress, address(0), "There is a match because it's a mutual like");
}
Updates

Appeal created

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
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.