Description
When a user executes the LikeRegistry::likeUser
function passing the address of the liked
user, the value of the LikeRegistry::userBalances
corresponding to the liked
user is not incremented by 1 ether
.
Impact
This results in zero funds being transferred to the created MultiSigWallet
contract inside the LikeRegistry::matchRewards
function if the like is mutual (i.e., the match is made).
Proof of Concept
-
User (say Bob) executes the LikeRegistry::likeUser
function, passing the address of the liked
user (say Alice).
-
The liked
user (i.e., Alice) also executes the LikeRegistry::likeUser
function passing the address of the user (i.e., Bob) who liked them.
-
Now, since the like is mutual, when the liked
user (i.e., Alice) executes the LikeRegistry::likeUser
function, the likes[liked][msg.sender]
value will be true
, triggering the call to the LikeRegistry::matchRewards
function.
-
However, the value of userBalances[from]
and userBalances[to]
is still 0
, because it was not incremented inside the LikeRegistry::likeUser
function.
-
So, when the rewards
are being sent to the MultiSigWallet
contract (corresponding to the match), their value is 0
, resulting in zero funds being transferred.
Proof of Code
Since there isn't a way to track the MultiSigWallet
contract address corresponding to the match, therefore I've added the return
statements in the LikeRegistry::likeUser
and LikeRegistry::matchRewards
functions, basically a kind of mutation testing.
So, the revised LikeRegistry::likeUser
and LikeRegistry::matchRewards
functions would be:
function likeUser(address liked) external payable returns(address matchToMultiSigWallet) {
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);
matchToMultiSigWallet = matchRewards(liked, msg.sender);
}
}
function matchRewards(address from, address to) internal returns(address) {
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");
return address(multiSigWallet);
}
Then, create a test file named as DatingDappTest.t.sol
inside the test
folder with the following content:
pragma solidity 0.8.20;
import {Test} from "forge-std/Test.sol";
import {LikeRegistry} from "../src/LikeRegistry.sol";
import {SoulboundProfileNFT} from "../src/SoulboundProfileNFT.sol";
import {MultiSigWallet} from "../src/MultiSig.sol";
contract DatingDappTest is Test {
SoulboundProfileNFT soulboundNFT;
LikeRegistry likeRegistry;
address owner = makeAddr("owner");
address bob = makeAddr("bob");
address alice = makeAddr("alice");
function setUp() public {
vm.startPrank(owner);
soulboundNFT = new SoulboundProfileNFT();
likeRegistry = new LikeRegistry(address(soulboundNFT));
vm.stopPrank();
}
function testMultiSigIsNotGettingFundedOnMatch() public {
vm.prank(bob);
soulboundNFT.mintProfile("Bob", 25, "ipfs://profileImage");
vm.prank(alice);
soulboundNFT.mintProfile("Alice", 24, "ipfs://profileImage");
hoax(bob, 1 ether);
address multiSigForMatch = likeRegistry.likeUser{value: 1 ether}(alice);
assertEq(multiSigForMatch, address(0), "There is no match because it's only one-sided like till now");
hoax(alice, 1 ether);
multiSigForMatch = likeRegistry.likeUser{value: 1 ether}(bob);
assertNotEq(multiSigForMatch, address(0), "There is a match because it's a mutual like now");
assertEq(multiSigForMatch.balance, 0);
}
}
Recommended Mitigation
To prevent this, the value of LikeRegistry::userBalances
mapping should be incremented by 1 ether
in the LikeRegistry::likeUser
function.
+ uint256 constant REWARD_ON_LIKE = 1 ether;
function likeUser(address liked) external payable returns(address matchToMultiSigWallet) {
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;
+ userBalances[liked] += REWARD_ON_LIKE;
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);
matchToMultiSigWallet = matchRewards(liked, msg.sender);
}
}