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

Potential loss of fund when calling `SoulboundProfileNFT::burnProfile` or `SoulboundProfileNFT::blockProfile`

roccomania

Summary

When a user burns their profile or even when a user is blocked, their earnings in the LikeRegistry contract are lost forever, as no one can withdraw them.

Vulnerability Details

The SoulboundProfileNFT::burnProfile or SoulboundProfileNFT::blockProfile functions deletes the user's profile and associated data but does not handle any earnings linked to the profile. This results in locked funds that cannot be accessed by either the protocol or user. In the LikeRegistry::likeUser, before there can be matches or like, it checks that profileNFT.profileToToken(msg.sender) != 0, i.e the profile has a token Id associated to them. Deleting a profile in the SoulboundProfileNFT will make the function revert at those checks since they will be 0. Making funds get locked in the contract forever.

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

Since there is another vulnerability in the LikeRegistry::likeUser that does not update user balances, it will be impossible to write codes to prove this. But a scenario can be created.

  • Consider a user that has gotten many likes and funded about 10 ether.

  • Then the protocol blocks this user in SoulboundProfileNFT::blockProfile. No check was made if user has balance in LikeRegistry.

  • User tries to like other profile but the transaction reverts since the profile has already been deleted.

  • These funds are locked in the contract since there is no function to retrieve them.

Impact

  • Loss of user funds.

  • Reduced trust in the protocol.

Tools Used

  • Manual code review.

Recommendations

Depending on how the protocol wants to handle unspent funds.

  • If the protocol wants to takeover balances of users with burnt profiles or users that are blocked, they should transfer the user balance to themselves before completely executing the functions.

  • If the protocol wants to refund the user, then they should send directly to the user before deleting their profiles.
    Here is an implementation to refund user, whereby the LikeRegistry has an emergencyWithdrawBalance function that can only be called by the SoulboundProfileNFT. Call the LikeRegistry::emergencyWithdrawBalance before performing the profile deleting action.

function burnProfile() external {
uint256 tokenId = profileToToken[msg.sender];
require(tokenId != 0, "No profile found");
require(ownerOf(tokenId) == msg.sender, "Not profile owner");
​
// Withdraw earnings before burning
+ LikeRegistry likeRegistry = LikeRegistry(likeRegistryAddress);
+ likeRegistry.withdrawEarnings(msg.sender);
​
_burn(tokenId);
delete profileToToken[msg.sender];
delete _profiles[tokenId];
​
emit ProfileBurned(msg.sender, tokenId);
}
Updates

Appeal created

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

finding_blocking_or_burning_no_refund_balances_or_multisig

Likelihood: Low, burning with money in it would be a user mistake, and being blocked is Low. Impact: High, loss of funds

Support

FAQs

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