DatingDapp
First Flight #33
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: high
Invalid

[H-2] Incorrect Reward Distribution Due To Aggregated User Balances in the `LikeRegistry` contract.

shabihethsec

Summary

The LikeRegistry contract incorrectly tracks ETH deposits in the userBalances mapping, leading to unintended behavior when distributing rewards for mutual likes.

Vulnerability Details

The userBalances accumulates all ETH sent by a user across multiple likeUser calls, rather than keeping track of funds on a per-recipient basis. When a mutual like occurs, the entire balance of both users is used to calculate rewards, including funds from unreciprocated likes, which can lead to unfair fund allocation and unintended loss of funds.

Affected Function: 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;
uint256 rewards = totalRewards - matchingFees;
totalFees += matchingFees;
​
// Deploy a MultiSig contract for the matched users
MultiSigWallet multiSigWallet = new MultiSigWallet(from, to);
​
// Send ETH to the deployed multisig wallet
(bool success, ) = payable(address(multiSigWallet)).call{value: rewards}("");
require(success, "Transfer failed");
}

Explanation of the Issue :

  1. Cumulative Tracking: The userBalances mapping accumulates ETH sent by a user across all likeUser calls, but does not differentiate between different recipients.

  2. Incorrect Reward Calculation: When a mutual like occurs, matchRewards resets the entire balance of both users (userBalances[from] and userBalances[to]), even though some of these funds may have been intended for other users who did not reciprocate the like.

  3. Loss of Funds: A user who has liked multiple people but received only one reciprocated like will have all their ETH used for rewards to the matched pair, potentially leaving other likes underfunded.

Example Scenario:

  1. Alex likes Rose (sends 1 ETH, userBalances[Alex] = 1)

  2. Alex likes Jamie (sends 1 ETH, userBalances[Alex] = 2)

  3. Jamie likes Alex (mutual like triggers matchRewards)

  4. The contract uses userBalances[Alex] = 2 ether and userBalances[Jamie] = 1 for rewards, including the ETH Alex sent for Rose

Result: The 1 ETH Alex intended for Rose is incorrectly allocated to Jamie. Rose will not receive the correct reward if he later reciprocates Alex’s like.

Impact

• Financial Loss: Users may lose funds intended for other potential matches.
• Unfair Reward Distribution: Some users receive more ETH than expected, while others lose their funds.
• Misleading Accounting: Users cannot correctly track or withdraw ETH allocated for specific likes.

POC:

How To Run This PoC:

Step 1: Update the likeUser function to store the balances of the user by adding the following statement after the checks:

userBalances[msg.sender] += msg.value;

This is also a vulnerability for which I have already submitted a bug report.

Step 2: Add getOwners() function in the MultiSigWallet contract:

function getOwners() external view returns(address _owner1, address _owner2) {
_owner1 = owner1;
_owner2 = owner2;
return (_owner1, _owner2);
}

Step 3: Declare the MultiSigWallet outside the function matchRewards and make this public, then use it to create new MultiSigWallet.

MultiSigWallet public multiSigWallet;
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;
​
// Deploy a MultiSig contract for the matched users
multiSigWallet = new MultiSigWallet(from, to);
​
// Send ETH to the deployed multisig wallet
(bool success, ) = payable(address(multiSigWallet)).call{value: rewards}("");
require(success, "Transfer failed");
}

Here is the PoC code to demonstrate the issue using foundry:

pragma solidity ^0.8.19;
​
import { Test, console } from "lib/forge-std/src/Test.sol";
import {Vm} from "lib/forge-std/src/Vm.sol";
import { LikeRegistry } from "../src/LikeRegistry.sol";
import { MultiSigWallet } from "src/MultiSig.sol";
import { SoulboundProfileNFT} from "src/SoulboundProfileNFT.sol";
​
contract LikeRegistryTest is Test {
LikeRegistry likeRegistry;
MultiSigWallet multiSig;
SoulboundProfileNFT profileNft;
​
uint256 public constant STARTING_USER_BALANCE = 10 ether;
address public USER1 = makeAddr("user1");
address public USER2 = makeAddr("user2");
address public USER3 = makeAddr("user3");
​
function setUp() external {
profileNft = new SoulboundProfileNFT();
likeRegistry = new LikeRegistry(address(profileNft));
​
vm.deal(USER1, STARTING_USER_BALANCE);
vm.deal(USER2, STARTING_USER_BALANCE);
vm.deal(USER3, STARTING_USER_BALANCE);
}
​
modifier nftMintedForAllTheUsers() {
vm.prank(USER1);
profileNft.mintProfile("Alex", 21, "ipfs://QmUPjADFGEKmfohdTaNcWhp7VGk26h5jXDA7v3VtTnTLcW");
vm.prank(USER2);
profileNft.mintProfile("Rose", 20, "ipfs://RandomSampleURI");
vm.prank(USER3);
profileNft.mintProfile("Jamy", 36, "ipfs://oiasdifulgxzvl0");
_;
}
function test_IncorrectRewardDistributionDueToAggregatedUserBalances() public nftMintedForAllTheUsers{
vm.prank(USER1);
likeRegistry.likeUser{value: 1 ether}(USER2);
vm.prank(USER1);
likeRegistry.likeUser{value: 1 ether}(USER3);
vm.prank(USER3);
likeRegistry.likeUser{value: 1 ether}(USER1);
// balance transferred in the wallet is 2.7 ether (1.8 ether from USER1 and 0.9 from USER3)
console.log("Wallet Balance: ", address(likeRegistry.multiSigWallet()).balance);
​
// check the owners of the MultiSigWallet i.e. USER1 and USER3
(address actualOwner3, address actualOwner4) = likeRegistry.multiSigWallet().getOwners();
assertEq(USER1, actualOwner3);
assertEq(USER3, actualOwner4);
}
}

Use this command to run the Poc:

forge test -vv

Tools Used

  • Manual Review

  • Forge Test

Recommendations

Step 1: Maintain a separate mapping to track ETH set per recipient:

Mapping(address => mapping(address => uint256)) public userSentBalances;

Step 2: Update likeUser to store ETH sent per liked address:

userSentBalances[msg.sender][liked] += msg.value;
​

Step 3: Modify matchRewards to use only the ETH sent specifically for the reciprocated like:

uint256 matchUserOne = userSentBalance[from][to];
uint256 matchUserTwo = userSentBalance[to][from];
userSentBalances[from][to] = 0;
userSentBalances[to][from] = 0;
Updates

Appeal created

n0kto Lead Judge 6 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.