DatingDapp

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

Denial Of Service in LikeRegistry::likeUser function as User Balances are Not Populated . Users can't access the funds when matched.

Summary:

LikeRegistry main function likeUser LikeRegistry::likeUserdoesn't save user balances when a user likes another user nft profile. This leads to not tracking the users funds and they won't be able to withdraw when they match.

Vulnerability Details:

LikeRegistry::likeUser User balances are not populated with the eth amount sent to the contract as likeUser is called. LikeRegistry contract tracks users funds in the storage variable userBalances. This variable is not

Impact:

Matched users are not able to access their funds. Contract owner is unable to access fees as the fees are calculated from the user balances as they are withdrawing.

Tools Used:

Manual review + vscodim

Proof Of code:

Add this test in SoulboundProfileNFT.t.sol and run the test

function testFeeWithdrawal() public {
vm.deal(user, 2 ether);
vm.deal(user2, 2 ether);
vm.prank(user);
likeRegistry.likeUser{value: 1 ether}(address(user2));
vm.prank(user2);
likeRegistry.likeUser{value: 1 ether}(address(user));
vm.prank(user2);
assertEq(likeRegistry.getMatches()[0], address(user), "User should be matched");
uint256 pre_balance = address(owner).balance;
vm.prank(owner);
likeRegistry.withdrawFees();
assertTrue(address(owner).balance > pre_balance, "Owner should receive fees");
}

You get this types of result::

forge test --mt testFeeWithdrawal
[⠒] Compiling...
No files changed, compilation skipped
Ran 1 test for test/testLikeRegistry.t.sol:LikeRegistryTest
[FAIL: revert: No fees to withdraw] testFeeWithdrawal() (gas: 714043)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 1.07ms (177.37µs CPU time)
Ran 1 test suite in 7.15ms (1.07ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/testLikeRegistry.t.sol:LikeRegistryTest
[FAIL: revert: No fees to withdraw] testFeeWithdrawal() (gas: 714043)

Recommendations:

userBalances[msg.sender] += msg.value; should be add in the LikeRegistry::likeUser as below. This will fix this vulnerability

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[msg.sender] += msg.value;
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);
}
}
Updates

Appeal created

n0kto Lead Judge 7 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.