DatingDapp

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

Users balances are not transferred to the created `MultiSigWallet` after they mutually like each other. Therefore, the funds are stuck in the `LikeRegistry` contract.

Description:

As we understand from the protocol's functioning, for a user to like another user, they must send a payment of at least 1 ether to the LikeRegistry::likeUser function during the liking process. When two users mutually like each other, a new MultiSigWallet is expected to be created, and the ether amounts sent by the users should be transferred to this newly created contract. However, after the MultiSigWallet is created, the funds are not transferred there. In fact, they are not even added to the LikeRegistry::userBalances array, where user balances are supposed to be stored.

Impact:

Since there is no withdrawal mechanism, the funds remain stuck in the contract indefinitely, breaking the entire functionality of the platform.

Proof of Concept:

First we are defining WalletAddress in LikeRegistry contract.

contract LikeRegistry is Ownable {
struct Like {
address liker;
address liked;
uint256 timestamp; // q timestamp defined in struct but never used. is that okay?
}
SoulboundProfileNFT public profileNFT;
uint256 immutable FIXEDFEE = 10;
uint256 totalFees;
+ address walletAddress;

After that, add this following line into LikeRegistry::matchRewards.

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; // @audit we don't like magic numbers.
uint256 rewards = totalRewards - matchingFees;
totalFees += matchingFees;
MultiSigWallet multiSigWallet = new MultiSigWallet(from, to);
+ walletAddress = address(multiSigWallet);

Now we can add this test function into testSoulboundProfileNFT.t.sol

function test_UsersMatchedBalancesTransferedToMultisigWallet() public {
vm.prank(user);
vm.deal(user, STARTING_BALANCE);
vm.deal(user2, STARTING_BALANCE);
soulboundNFT.mintProfile("alice", 25, "ipfs://profileImage");
vm.prank(user2);
soulboundNFT.mintProfile("bob", 22, "ipfs://profileImage");
vm.prank(user);
likeRegistry = new LikeRegistry(address(soulboundNFT));
console.log("alice TokenId:", soulboundNFT.profileToToken(user));
console.log("bob TokenId:", soulboundNFT.profileToToken(user2));
vm.prank(user);
console.log("alice Balance before Like:", address(user).balance);
likeRegistry.likeUser{value: 1 ether}(user2);
console.log("alice likes bob");
console.log(
"likeRegistry Contract Balance:",
address(likeRegistry).balance
);
console.log("alice Balance after like:", address(user).balance);
vm.prank(user2);
console.log("bob Balance before Like:", address(user2).balance);
likeRegistry.likeUser{value: 1 ether}(user);
console.log("bob likes back alice");
console.log("bob Balance after Like:", address(user2).balance);
address WalletAddress = likeRegistry.getWalletAddress();
console.log("Users Matched. Wallet address:", WalletAddress);
console.log(
"Balance of MultiSig Wallet Address:",
WalletAddress.balance
);
console.log(
"likeRegistry Contract Balance after match:",
address(likeRegistry).balance
);
assert(WalletAddress.balance > 0);
}

Recommended Mitigation:

We need to add the amount of ether sent by users when they like each other to the LikeRegistry::userBalances array. This way, when the LikeRegistry::likeUser function calls the LikeRegistry::MatchRewards function, the LikeRegistry::userBalances will be transferred to the created MultiSigWallet address. Also, we must add a withdraw function for the scenario where one user likes another, but the other user does not likes back. This way, we can prevent the funds of platonic users from getting stuck in the function.

Add this line into LikeRegistry::likeUser function.

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

Add this function to LikeRegistry.

function withdraw(uint256 _amount) external {
uint256 userBalance = userBalances[msg.sender];
// Check amount and user balance.
require(_amount > 0, "Withdraw amount must be bigger than 0.");
require(userBalance >= _amount, "Insufficient user balance.");
// Update the user balance.
balances[msg.sender] -= _amount;
// Send ETH.
(bool success, ) = msg.sender.call{value: _amount}("");
require(success, "ETH transfer failed.");
}
Updates

Appeal created

n0kto Lead Judge 6 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.