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

Improper Handlng Of Eth, Leading To Locked User Funds

kvltbyte

Description

In the LikeRegistry::likeUser function, users can like a profile by sending ETH to the contract. However, the contract fails to track individual user balances in the LikeRegistry::userBalances mapping, which is intended to store the ETH sent by each user. As a result, the mapping remains unupdated, and user balances are always zero.

Impact

  1. Loss of Protocol Revenue: The LikeRegistry::totalFees will always be zero because the matchRewards function calculates fees based on LikeRegistry::userBalances, which are never updated. This results in the protocol losing all potential revenue from matching fees.

  2. Zero ETH Sent to Multisig Wallet: The LikeRegistry::matchRewards function sends rewards to the multisig wallet based on LikeRegistry::userBalances. Since these balances are always zero, no ETH is ever sent to the multisig wallet, rendering the matching feature useless.

  3. Locked User Funds: The ETH sent by users remains locked in the contract indefinitely, as there is no mechanism to track or refund it.

Tools Used

  • Manual code review.

  • Foundry for testing (as demonstrated in the PoC).

Proof Of Concept

  1. User A likes User B's profile by sending 1 ETH.

  2. The balance of User A in LikeRegistry::userBalances is expected to be 1 ETH.

  3. However, due to the current implementation, User A's balance is never incremented and remains zero.

  4. Add the following test case to confirm this behaviour:

function testImproperHandlingOfEth() public {
vm.deal(user2, 1 ether);
vm.deal(user, 1 ether);
​
vm.prank(user);
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
​
vm.startPrank(user2);
soulboundNFT.mintProfile("Bob", 30, "ipfs://profileImage");
likeRegistry.likeUser{value: 1 ether}(user);
vm.stopPrank();
​
vm.prank(user);
likeRegistry.likeUser{value: 1 ether}(user2);
​
assertEq(likeRegistry.userBalances(user2), 0);
assertEq(likeRegistry.userBalances(user), 0);
vm.prank(owner);
vm.expectRevert("No fees to withdraw");
likeRegistry.withdrawFees();
​
}

Recommended Mitigations

Update user balances when a user likes a profile in LikeRegistry::likeUser:

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] += 1 ether;
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);
}
}
Updates

Appeal created

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