Submission Details
Severity:
A User that was matched earlier and has an active MultiSignature wallet can still get matched with another user and own multiple wallets.
Summary
In the `LikeRegistry::likeUser`, we do not check if the user has an active MultiSig wallet already. The user can still be matched with other users even if they already have multiple multisignature wallets. As a dating application, I feel there should be a level of fairness and one user should not be able to pair with multiple users.
Vulnerability Details
Proof of Concept:
1. UserA likes UserB and UserC profile.
2. UserB and UserC see UserA profile and decide to reciprocate the like at different times.
3. 2 multisignature wallets are created. One for UserA&B and one for UserA&C. Hence, UserA has access to 2 wallets while UserB and UserC only have access to one wallet each.
Proof of Code:
<details>
<summary>Code</summary>
Add the following code to the `testSoulboundProfileNFT.t.sol` file.
```javascript
// Add the following code to the `LikeRegistry.sol` file for the test to run.
function likeUser(address liked) external payable returns (MultiSigWallet multiSigWallet) {
//user can send any amount, why?
require(msg.value >= 1 ether, "Must send at least 1 ETH");
//@ochuko -> potential bug
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[liked] += msg.value;
likes[msg.sender][liked] = true;
emit Liked(msg.sender, liked);
if (likes[liked][msg.sender]) {
matches[msg.sender].push(liked);
matches[liked].push(msg.sender);
emit Matched(msg.sender, liked);
multiSigWallet = matchRewards(liked, msg.sender);
}
return multiSigWallet;
}
function matchRewards(address from, address to) internal returns (MultiSigWallet) {
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, address(this));
// Send ETH to the deployed multisig wallet
(bool success,) = payable(address(multiSigWallet)).call{value: rewards}("");
require(success, "Transfer failed");
return multiSigWallet;
}
function testUserCanOwnMultipleActiveMultiSigWallets() public {
LikeRegistry userRegistry = new LikeRegistry(address(soulboundNFT));
vm.prank(user); // Simulates user calling the function
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
vm.prank(user2); // Simulates user2 calling the function
soulboundNFT.mintProfile("Ochuko", 24, "ipfs://profileImage");
vm.prank(user3); // Simulates user2 calling the function
soulboundNFT.mintProfile("Kevwe", 26, "ipfs://profileImage");
vm.deal(user, 3 * TRANSFER_AMOUNT);
vm.deal(user2, 3 * TRANSFER_AMOUNT);
vm.deal(user3, 3 * TRANSFER_AMOUNT);
//User likes user2
vm.prank(user);
userRegistry.likeUser{value: TRANSFER_AMOUNT}(user2);
//User also likes user3
vm.prank(user);
userRegistry.likeUser{value: TRANSFER_AMOUNT}(user3);
vm.warp(block.timestamp + 7200);
//User2 likes user back
vm.prank(user2);
MultiSigWallet multiSigWallet1 = userRegistry.likeUser{value: TRANSFER_AMOUNT}(user);
vm.warp(block.timestamp + 7200);
//User3 likes user back
vm.prank(user3);
MultiSigWallet multiSigWallet2 = userRegistry.likeUser{value: TRANSFER_AMOUNT}(user);
assertEq(multiSigWallet1.owner1(), user);
assertEq(multiSigWallet2.owner1(), user);
}
```
</details>
Impact
This will beat the whole purpose of the system. As one user, the one who gets matched multiple times gets to gain the bulk of many people's fund.
Recommendations
To fix this, we can add a mapping that tracks user addresses and their active multiSignature wallet(wallet with balance greater than zero) as a user should only have one active multiSignature wallet for the application to be fair.
```diff
//add a mapping for user address to multisignature wallet address
+ mapping(address => address) public walletToOwner;
function likeUser(address liked) external payable returns (MultiSigWallet multiSigWallet) {
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[liked] += msg.value;
likes[msg.sender][liked] = true;
emit Liked(msg.sender, liked);
if (likes[liked][msg.sender]) {
+ require(walletToOwner[liked].balance == 0 && walletToOwner[msg.sender].balance == 0, 'One user already has an active multiSignature wallet');
matches[msg.sender].push(liked);
matches[liked].push(msg.sender);
emit Matched(msg.sender, liked);
multiSigWallet = matchRewards(liked, msg.sender);
+ walletToOwner[liked] = address(multiSigWallet);
+ walletToOwner[msg.sender] = address(multiSigWallet);
}
return multiSigWallet;
}
function matchRewards(address from, address to) internal returns (MultiSigWallet) {
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;
MultiSigWallet multiSigWallet = new MultiSigWallet(from, to, address(this));
// Send ETH to the deployed multisig wallet
(bool success,) = payable(address(multiSigWallet)).call{value: rewards}("");
require(success, "Transfer failed");
return multiSigWallet;
}
```
Rewards breakdown
nSLOC
197This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.