Summary
Funds will get locked in the contract because user balances are not properly updated.
Vulnerability Details
In the LikeRegistry::likeUser function, msg.value is never added to the userBalances. The sent ETH will go into the contract balance but as it is not recorded in userBalances, this will later cause LikeRegistry::matchRewards to send effectively 0 to the multi sig wallet and the totalFees will also remain 0:
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;
Proof of Concept
Here is a test that presents a case where both Alice and Bob create profiles, like each other and get matched per mutual like. However, while the contract balance is now 2 ether, the totalFees is not 0.2 ETH but still 0.
pragma solidity ^0.8.19;
import "forge-std/Test.sol";
import "../src/LikeRegistry.sol";
import "../src/SoulboundProfileNFT.sol";
contract LikeRegistryTest is Test {
LikeRegistry likeRegistry;
SoulboundProfileNFT profileNFT;
address alice = address(0x1);
address bob = address(0x2);
receive() external payable {}
function setUp() public {
profileNFT = new SoulboundProfileNFT();
likeRegistry = new LikeRegistry(address(profileNFT));
vm.deal(alice, 2 ether);
vm.deal(bob, 2 ether);
vm.prank(alice);
profileNFT.mintProfile("Alice", 25, "ipfs://alice");
vm.stopPrank();
vm.prank(bob);
profileNFT.mintProfile("Bob", 28, "ipfs://bob");
vm.stopPrank();
}
function testMatchRewardWithoutBalanceUpdate() public {
vm.prank(alice);
likeRegistry.likeUser{value: 1 ether}(bob);
vm.stopPrank();
assertTrue(likeRegistry.likes(alice, bob), "Alice's like should be recorded");
assertEq(likeRegistry.userBalances(alice), 0, "Alice's balance should be 0 as it's not updated");
vm.prank(bob);
likeRegistry.likeUser{value: 1 ether}(alice);
vm.stopPrank();
assertTrue(likeRegistry.likes(bob, alice), "Bob's like should be recorded");
assertEq(likeRegistry.userBalances(bob), 0, "Bob's balance should be 0 as it's not updated");
vm.prank(alice);
address[] memory aliceMatches = likeRegistry.getMatches();
assertEq(aliceMatches.length, 1, "Should have one match now");
assertEq(aliceMatches[0], bob, "Should be matched with Bob");
vm.stopPrank();
assertEq(likeRegistry.totalFees(), 0, "Total fees should be 0 since balances were not tracked properly");
assertEq(address(likeRegistry).balance, 2 ether, "Contract should hold 2 ETH from likes");
vm.prank(address(this));
vm.expectRevert("No fees to withdraw");
likeRegistry.withdrawFees();
vm.stopPrank();
}
}
Important note for this test: to be able to check likeRegistry.totalFees() make sure to modify it to be public in LikeRegistry.sol first:
uint256 public totalFees;
Impact
This will cause funds to get locked in the contract and the couple will be left with no funds in their multi sig wallet on their date.
Tools Used
Manual inspection, copilot and foundry.
Recommended Mitigation
The simple solution is to properly update userBlances in 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");
likes[msg.sender][liked] = true;
emit Liked(msg.sender, liked);
userBalances[msg.sender] += msg.value;
