Description: The state variable LikeRegistry::userBalances is intended to track balances for each user of the contract. However, in the function LikeRegistry::LikeUser, the calling user's balance is not updated to reflect the addition of 1 ETH. This oversight causes a complete breakdown of the protocol since any funds paid to the contract are not being tracked. Consequently, when users like each other, a shared multi-signature wallet is created, but it remains empty. Additionally, the owner of the LikeRegistry contract is unable to withdraw fees since all user balances are zero.
Impact: All user funds are locked in the contract and the owner can't withdraw fees either.
Proof of Concept:
Add the file testLikeRegistry.t.sol to the tests folder and copy the following code into the source file:
pragma solidity ^0.8.19;
import {Test, console} from "forge-std/Test.sol";
import {LikeRegistry} from "../src/LikeRegistry.sol";
import {SoulboundProfileNFT} from "../src/SoulboundProfileNFT.sol";
contract LikeRegistryTest is Test {
LikeRegistry likeRegistry;
SoulboundProfileNFT profileNFT;
address user1 = makeAddr("User1");
address user2 = makeAddr("User2");
address owner = address(this);
function setUp() public {
profileNFT = new SoulboundProfileNFT();
likeRegistry = new LikeRegistry(address(profileNFT));
assertEq(likeRegistry.owner(), address(this));
vm.prank(user1);
profileNFT.mintProfile("User1", 25, "ipfs://profileImage");
vm.prank(user2);
profileNFT.mintProfile("User2", 30, "ipfs://profileImage");
}
function testUserBalancesNotUpdated() public {
vm.deal(user1, 1 ether);
vm.deal(user2, 1 ether);
console.log("Balance of LikeRegistry:", address(likeRegistry).balance);
console.log("LikeRegistry balance of user1:", likeRegistry.userBalances(user1));
console.log("User1 likes User2...");
vm.prank(user1);
likeRegistry.likeUser{value: 1 ether}(user2);
console.log("LikeRegistry balance of user1:", likeRegistry.userBalances(user1));
console.log("Balance of LikeRegistry:", address(likeRegistry).balance);
assertEq(likeRegistry.userBalances(user1), 0 ether);
console.log("User2 likes User1...");
vm.prank(user2);
likeRegistry.likeUser{value: 1 ether}(user1);
assertEq(address(likeRegistry).balance, 2 ether);
console.log("Balance of LikeRegistry:", address(likeRegistry).balance);
vm.prank(owner);
vm.expectRevert("No fees to withdraw");
likeRegistry.withdrawFees();
}
}
Run the unit test with verbosity: forge test --mt testUserBalancesNotUpdated -vvv
The unit test will pass and the following is logged to the screen:
Logs:
Balance of LikeRegistry: 0
LikeRegistry balance of user1: 0
User1 likes User2...
LikeRegistry balance of user1: 0
Balance of LikeRegistry: 1000000000000000000
User2 likes User1...
Balance of LikeRegistry: 2000000000000000000
Recommended Mitigation: Track user balances by updating the state variable LikeRegistry::userBalances in the function LikeRegistry::LikeUser by adding 1 ETH for msg.sender.
function likeUser(
address liked
) external payable {
require(msg.value >= 1 ether, "Must send at least 1 ETH"); // @audit-XXX how do you get overpayment back?
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;
+ userBalances[msg.sender] += 1 ether;
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);
}
}
