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];
require(_amount > 0, "Withdraw amount must be bigger than 0.");
require(userBalance >= _amount, "Insufficient user balance.");
balances[msg.sender] -= _amount;
(bool success, ) = msg.sender.call{value: _amount}("");
require(success, "ETH transfer failed.");
}