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

The Ether sent during the `LikeRegistry::likeUser` function call is not added to the userBalances mapping, causing loss of fund

roccomania

Summary

The LikeRegistry contract contains a vulnerability where the msg.value (Ether) sent during the LikeRegistry::likeUser() function call is not added to the userBalances mapping. This results in the funds being held in the contract's balance but not being properly allocated to the user's balance, which can lead to incorrect reward distribution and potential loss of funds.

Vulnerability Details

The vulnerability is present in the likeUser function. When a user sends ETH to like another user, the msg.value is not recorded in the userBalances mapping. This means that even though the ETH is sent to the contract, it is not attributed to the user's balance, which is later used in the matchRewards function to distribute rewards.

Code Snippet

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");
​
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);
}
}

Proof of Concept

Explanation

The PoC demonstrates the exploit by simulating the following steps:

  • Two users, User1 and User2, minted NFT profiles.

  • User1 liked User2 by calling LikeRegistry::likeUser() and paid 1 Ether to execute the transaction.

  • User2 balance in the contract was not updated but the contract balance increased.

The provided test case testNoEthDepositRecorded demonstrates the issue. After USER1 likes USER2 and deposits 1 ETH, the userBalances mapping for both users remains 0, even though the contract's balance increases by 1 ETH.

Code

Here is the code the prove the scenario. Create a new test file LikeRegistryTest.t.sol and add to the test/ directory.

Then run the command below;

forge test --mt test_NoEthDepositRecorded
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
​
import {LikeRegistry} from "../src/LikeRegistry.sol";
import {SoulboundProfileNFT} from "../src/SoulboundProfileNFT.sol";
import {console, Test} from "forge-std/Test.sol";
​
contract LikeRegistryTest is Test {
SoulboundProfileNFT soulboundNFT;
LikeRegistry likeRegistry;
address user1 = makeAddr("USER1");
address user2 = makeAddr("USER2");
uint256 constant STARTING_BALANCE = 100 ether;
uint256 constant DEPOSIT_AMOUNT = 1 ether;
​
function setUp() public {
soulboundNFT = new SoulboundProfileNFT();
likeRegistry = new LikeRegistry(address(soulboundNFT));
vm.deal(user1, STARTING_BALANCE);
vm.deal(user2, STARTING_BALANCE);
}
​
//Modifier to handle minting profile for both users
modifier mintBothUsers() {
vm.prank(user1);
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
vm.prank(user2);
soulboundNFT.mintProfile("Bob", 29, "ipfs://BobprofileImage");
uint256 user1TokenId = soulboundNFT.profileToToken(user1);
assertEq(user1TokenId, 1, "Token ID for user 1 should be 1");
uint256 user2TokenId = soulboundNFT.profileToToken(user2);
assertEq(user2TokenId, 2, "Token ID for user 2 should be 2");
_;
}
​
function test_NoEthDepositRecorded() public mintBothUsers {
//USER 1 liked a user and deposited 2 ether
vm.prank(user1);
likeRegistry.likeUser{value: DEPOSIT_AMOUNT}(user2);
uint256 likeRegistryEtherBalance = address(likeRegistry).balance;
uint256 user1Balance = likeRegistry.userBalances(user1);
uint256 user2Balance = likeRegistry.userBalances(user2);
​
//Assert that Like Registry Ether balance is equal to the deposited Ether amount
assertEq(likeRegistryEtherBalance, DEPOSIT_AMOUNT);
​
//Assert that USER1 balance is 0
assertEq(user1Balance, 0);
//Assert that USER2 balance is 0
assertEq(user2Balance, 0);
}
}
​

Impact

The impact of this vulnerability is significant:

  • Loss of funds since the contract's balance is not managed correctly and no way to withdraw them.

  • The contract's intended functionality of rewarding mutual likes is broken.

  • Misallocation of rewards during mutual likes, as the matchRewards function relies on the userBalances mapping.

Tools Used

  • Manual review

  • Foundry (Forge) for testing and vulnerability demonstration.

Recommendations

To fix this vulnerability, the msg.value should be added to the userBalances mapping of the user who was liked. This can be done by updating the likeUser function as follows:

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");
​
likes[msg.sender][liked] = true;
+ userBalances[liked] += msg.value; // Add msg.value to userBalances
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);
matchRewards(liked, msg.sender);
}
}
Updates

Appeal created

n0kto Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_likeUser_no_userBalances_updated

Likelihood: High, always. Impact: High, loss of funds

Support

FAQs

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