Summary
LikeRegistry::matchRewards resets the matched users' userBalances. However, if a previously matched user gets liked and matched later on, the share multisig wallet will contain no funds from him. Technically, this scenario isn't possible due to a bug where userBalances is never updated with user funds, but the flawed logic is still there.
Vulnerability Details
Upon a match, LikeRegistry::matchRewards gets called.
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");
}
matchRewards will collect ([2]) all their previous like payments (minus a 10% fee [3]) and finally create a shared multisig wallet between the two users, which they can access for their first date ([4]). Note how userBalances is reset at [1]. Imagine the following scenario:
bob likes alice
bob likes angie
alice likes bob (match)
angie likes alex
angie likes tony
angie likes bob (match)
shared multisig wallet is created using only angie's funds
Proof of Concept
To demonstrate the aforementioned scenario, apply the following patch in LikeRegistry::likeUser in order to update userBalances properly,
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);
}
}
as well as a few logs in LikeRegistry::matchRewards:
function matchRewards(address from, address to) internal {
uint256 matchUserOne = userBalances[from];
uint256 matchUserTwo = userBalances[to];
@> console.log("[LikeRegistry::matchRewards] matchUserOne:", matchUserOne);
@> console.log("[LikeRegistry::matchRewards] matchUserTwo:", matchUserTwo);
userBalances[from] = 0;
userBalances[to] = 0;
MultiSigWallet multiSigWallet = new MultiSigWallet(payable(address(this)), from, to);
(bool success, ) = payable(address(multiSigWallet)).call{
value: rewards
}("");
require(success, "Transfer failed");
@> console.log("[LikeRegistry::matchRewards] multiSigWallet balance:", address(multiSigWallet).balance);
}
Finally, place test_UserCanGetFreeDates in testSoulboundProfileNFT.t.sol:
function test_UserCanGetFreeDates() public {
address bob = makeAddr("bob");
address alice = makeAddr("alice");
address angie = makeAddr("angie");
address alex = makeAddr("alex");
address tony = makeAddr("tony");
vm.deal(bob, 10 ether);
vm.deal(alice, 10 ether);
vm.deal(angie, 10 ether);
vm.prank(bob);
soulboundNFT.mintProfile("Bob", 25, "ipfs://profileImage");
vm.prank(alice);
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
vm.prank(angie);
soulboundNFT.mintProfile("Angie", 25, "ipfs://profileImage");
vm.prank(alex);
soulboundNFT.mintProfile("Alex", 25, "ipfs://profileImage");
vm.prank(tony);
soulboundNFT.mintProfile("Tony", 25, "ipfs://profileImage");
vm.prank(bob);
likeRegistry.likeUser{value: 1 ether}(alice);
assertTrue(likeRegistry.likes(bob, alice));
vm.prank(bob);
likeRegistry.likeUser{value: 1 ether}(angie);
assertTrue(likeRegistry.likes(bob, angie));
console.log("====== FIRST MATCH ======");
vm.prank(alice);
likeRegistry.likeUser{value: 1 ether}(bob);
assertTrue(likeRegistry.likes(alice, bob));
vm.prank(angie);
likeRegistry.likeUser{value: 1 ether}(alex);
assertTrue(likeRegistry.likes(angie, alex));
vm.prank(angie);
likeRegistry.likeUser{value: 1 ether}(tony);
assertTrue(likeRegistry.likes(angie, tony));
console.log("\n\n====== SECOND MATCH ======");
vm.prank(angie);
likeRegistry.likeUser{value: 1 ether}(bob);
}
and run the test:
$ forge test --mt test_UserCanGetFreeDates -vvv
Ran 1 test for test/testSoulboundProfileNFT.t.sol:SoulboundProfileNFTTest
[PASS] test_UserCanGetFreeDates() (gas: 2274150)
Logs:
====== FIRST MATCH ======
[LikeRegistry::matchRewards] matchUserOne: 2000000000000000000
[LikeRegistry::matchRewards] matchUserTwo: 1000000000000000000
[LikeRegistry::matchRewards] totalFees: 300000000000000000
[LikeRegistry::matchRewards] multiSigWallet balance: 2700000000000000000
====== SECOND MATCH ======
[LikeRegistry::matchRewards] matchUserOne: 0
[LikeRegistry::matchRewards] matchUserTwo: 3000000000000000000
[LikeRegistry::matchRewards] totalFees: 600000000000000000
[LikeRegistry::matchRewards] multiSigWallet balance: 2700000000000000000
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 7.57ms (1.93ms CPU time)
Ran 1 test suite in 139.45ms (7.57ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
Note how on the second match (between angie and bob), matchUserOne (which corresponds to bob) is 0. It was reset upon his match with alice.
Impact
Users can get free dates. The impact is low since this bug isn't technically feasible due to a bug in the LikeRegistry contract where userBalances isn't properly updated with user payments. The logic flaw remains though.
Tools used
Manual review, tests
Recommendations
Consider grouping ETH-like payments on a per-like basis instead of all together.
contract LikeRegistry is Ownable {
// ...
mapping(address => mapping(address => bool)) public likes;
mapping(address => address[]) public matches;
- mapping(address => uint256) public userBalances;
+ mapping(address => mapping(address => uint256)) public userBalances;
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][liked] += msg.value;
likes[msg.sender][liked] = true;
emit Liked(msg.sender, liked);
// ...
}
function matchRewards(address from, address to) internal {
- uint256 matchUserOne = userBalances[from];
- uint256 matchUserTwo = userBalances[to];
+ uint256 matchUserOne = userBalances[from][to];
+ uint256 matchUserTwo = userBalances[to][from];
- userBalances[from][to] = 0;
- userBalances[to][from] = 0;
+ userBalances[from][to] = 0;
+ userBalances[to][from] = 0;
// ...
}
}
