Summary
The LikeRegistry contract contains the LikeRegistry::likeUser function, which accepts Ether from the msg.sender to collect a fee and then creates a multisig wallet with a matching party. The vulnerability stems from a chain of issues:
The contract does not allow anyone, not even the owner, to withdraw the funds stored in the contract's balance.
The LikeRegistry::likeUser function fails to update the state variable userBalance. This is problematic because in the LikeRegistry::matchRewards function, the amount sent to the new multisig wallet will always be zero, meaning the Ether becomes trapped in the contract.
Vulnerability Details
The LikeRegistry::likeUser function is shown below:
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);
if (likes[liked][msg.sender]) {
matches[msg.sender].push(liked);
matches[liked].push(msg.sender);
emit Matched(msg.sender, liked);
matchRewards(liked, msg.sender);
}
}
The function fails to update the userBalance mapping, where the value deposited by a user is held. As a result, when the fee is calculated based on the balance of the new couple, it will always be zero.
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;
uint256 rewards = totalRewards - matchingFees;
totalFees += matchingFees;
MultiSigWallet multiSigWallet = new MultiSigWallet(from, to);
(bool success, ) = payable(address(multiSigWallet)).call{value: rewards}("");
require(success, "Transfer failed");
}
The PoC below demonstrates the issue:
function testLikeUserLockFunds() public {
vm.prank(user);
profileNFT.mintProfile("Alice", 25, "ipfs://profileImage");
vm.prank(user2);
profileNFT.mintProfile("Bob", 30, "ipfs://profileImage");
vm.deal(user, 5 ether);
vm.prank(user);
likeRegistry.likeUser{value: 2 ether}(user2);
assertTrue(likeRegistry.likes(user, user2), "User should like user2");
vm.deal(user2, 5 ether);
vm.prank(user2);
likeRegistry.likeUser{value: 1 ether}(user);
assertTrue(likeRegistry.likes(user2, user), "User2 should like user");
assertEq(likeRegistry.totalFees(), 0);
}
Ran 1 test for test/testLikeRegistry.sol:TestLikeRegistry
[PASS] testLikeUserLockFunds() (gas: 1020817)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 577.25µs (190.17µs CPU time)
Impact
Any ether sent to the LikeRegistry::likeUser function will be permanently locked inside the contract.
Tools Used
Manual Review
Recommendations
In order to mitigate this issue, the function LikeRegistry::likeUser must update the userBalance state variable:
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);
// 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);
}
}
This makes the fee calculation and the creation of the multisig wallet with the couple's funds correctly. Now, adjusting the test:
function testLikeUserLockFunds() public {
vm.prank(user);
profileNFT.mintProfile("Alice", 25, "ipfs://profileImage");
vm.prank(user2);
profileNFT.mintProfile("Bob", 30, "ipfs://profileImage");
vm.deal(user, 5 ether);
vm.prank(user);
likeRegistry.likeUser{value: 2 ether}(user2);
assertTrue(likeRegistry.likes(user, user2), "User should like user2");
vm.deal(user2, 5 ether);
vm.prank(user2);
likeRegistry.likeUser{value: 1 ether}(user);
assertTrue(likeRegistry.likes(user2, user), "User2 should like user");
+ assertEq(likeRegistry.totalFees(), 0.3 ether); // 10% of the deposited eth as expected.
- assertEq(likeRegistry.totalFees(), 0);
}
}
Ran 1 test for test/testLikeRegistry.sol:TestLikeRegistry
[PASS] testLikeUserLockFunds() (gas: 1020817)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 577.25µs (190.17µs CPU time)
