DatingDapp

First Flight #33
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Submission Details
Severity: medium
Valid

Missing funds to create more then one matches for one user.

Description

There is `LikeRegistry::matchRewards` function which retrive `LikeRegistry::userBalances` which is suppose to store the mappinf of users funds deposited in contract while like anyone.
​
```javascript
function matchRewards(address from, address to) internal {
uint256 matchUserOne = userBalances[from]; // q how to get user balance?// there is no update in this mapping.
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");
}
```
​
However this `LikeRegistry::userBalances` mapping is not updated anywhere for map the user funds it will alway be `zero`. But let's assume that `LikeRegistry::userBalances` updated properly in `LikeRegistry::like` function. Then we can assume that this will give us some value of user funds. After retriving user balance for both `liker` and `liked` it is immidiatly assigned to `zero`. And then after All the logic to calculate fees and reward will be done and `MultiSigWallet` will be created for both the user, and the reward will send to the wallet.
​
But this is an issue, suppose a `user` liked two diffrent `users` at diffrent time or same time. Amoung these two liked users `one user` also like the `liker` then as i explained the wallet will created and all the calculated reward will send to the wallet. But if you notice the user who liked at first his/her balance will be `zero` in mapping `LikeRegistry::userBalances`.
​
After some time `second user` from liked list of `liker` also like the `liker` then all the calculation will be done for these two user again and the wallet will be created. But if we see the `user` who like first his/her balance is `zero`. Then all the calculation will be perform on `second user's` balance. This will unfair to second user because only second user's fund will be calculated as reward instead of both `users`. This will lead to missing funds of the `liker` second time.
​
Also this will consider as a `cheat`, Like in point of view of `Dating` if you are dating someone and along side you are dating another one, Then it will called cheat on `both`.

Impact

User can take advantage of missing funds mapping, And this will lead to exploit of protocol.

Proof of Concept

Let's create a senario.
1. `Three` users are there in `LikeRegistry`
2. `user1` and `user2` and `user3` with some balance in their wallets
3. Suppose `user1` like `user2` and `user3` by sending `1 ETH` diffrently to the contract.
4. `user1's` `liked` list will be updated with liked ones.
5. Now contract balance is `2 ETH` And the mapping `userBalances` is also `2 ETH` eg. `userBalances[user1]` will store `2`
6. `user2` also like `user1` by sending `1 ETH` to the contract.
7. `matchRewards` function will called and the `userBalances` will be update. eg. `userBalances[user1]` will `zero` and `userBalances[user2]` also will `zero`
8. The calculated reward will be:
1. matchUserOne = 1
2. matchUserTwo = 1
3. totalRewards = 2
4. matchingFees = (2 * 10) / 100 = 0.2
5. reward = 2 - 0.2 = `1.8 ether`
9. Now wallet will be created an send the `1.8 ETH` to the created `MultiSigWallet`.
10. `user3` also like `user1` by sending the `1 ETH` to the contract.
.... all the calculation will be done again for this time.
​
But `userBalances[user1]` is `zero` after first matching. then acconrding to the logic only `user3's` fund will be calculated for reward.
​
The `reward` will be:
1. matchUserOne = 0
2. matchUserTwo = 1
3. totalRewards = 1
4. matchingFees = (1 * 10) / 100 = 0.1
5. reward = 2 - 0.1 = `1.9 ether`
​
Here `user1` has taken the advantage of the vulnerability. without calculating his/her funds.

Recommended Mitigation

The protocol should implement diffrent mapping for trase the user's funds.
​
```javascript
mapping (address => mapping (address => uint256)) public userBalances;
```
​
This will map the user funds associate with each user whome was liked by user, and do all the reward calculation on this mapping.
​
Also protocol should implemnent logic for checking if user has already matched wil someone or not.
​
```diff
+ mapping (address => bool) public matched;
```
​
Also add this suggestion.
​
```diff
function matchRewards(address from, address to) internal {
+ require(!matched[from] && !matched[to], "Users are already matched with someone");
uint256 matchUserOne = userBalances[from]; // q how to get user balance?// there is no update in this mapping.
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;
​
+ matched[from] = true;
+ matched[to] = true;
​
// 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");
}
```
Updates

Appeal created

n0kto Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_several_match_lead_to_multisig_with_no_funds

Likelihood: Medium, if anyone has 2 matches or more before reliking. Impact: Medium, the user won't contribute to the wallet.

Support

FAQs

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

Give us feedback!