DatingDapp

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

[H-3] Burning or blocking a user's profile leads to a loss of user funds

Description:

  • A user can burn their profile by calling SoulboundProfileNFT::burnProfile, which results in the user's profile/NFT being burned.

  • The owner of SoulboundProfileNFT can also block a user's profile by calling SoulboundProfileNFT::blockProfile, which also results in the user's profile/NFT being burned.

  • The issue is if the user whose profile is being burned or blocked has a user balance with LikeRegistry, then their funds are lost. There's no code to return the funds to the user.

Impact: A loss of funds for users with balances with the contract LikeRegistry when their profile is burned or blocked.

Proof of Concept:

Add the file testLikeRegistry.t.sol to the tests folder and copy the following code into the source file:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.19;
import {Test, console} from "forge-std/Test.sol";
import {LikeRegistry} from "../src/LikeRegistry.sol";
import {SoulboundProfileNFT} from "../src/SoulboundProfileNFT.sol";
contract LikeRegistryTest is Test {
LikeRegistry likeRegistry;
SoulboundProfileNFT profileNFT;
address user1 = makeAddr("User1");
address user2 = makeAddr("User2");
address owner = address(this); // Test contract acts as the owner
function setUp() public {
profileNFT = new SoulboundProfileNFT();
likeRegistry = new LikeRegistry(address(profileNFT));
assertEq(likeRegistry.owner(), address(this));
vm.prank(user1);
profileNFT.mintProfile("User1", 25, "ipfs://profileImage");
vm.prank(user2);
profileNFT.mintProfile("User2", 30, "ipfs://profileImage");
}
function testBlockProfileLossOfFunds() public {
vm.deal(user1, 1 ether);
console.log("Balance of user1:", user1.balance);
console.log("Balance of LikeRegistry:", address(likeRegistry).balance);
// User1 likes user2 so LikeRegistry has 1 ETH
console.log("User1 likes User2...");
vm.prank(user1);
likeRegistry.likeUser{value: 1 ether}(user2);
console.log("Balance of user1:", user1.balance);
console.log("Balance of LikeRegistry:", address(likeRegistry).balance);
// Owner blocks user1
console.log("Owner blocks user1...");
vm.prank(owner);
profileNFT.blockProfile(user1);
// Bug - User1 balance is not credited 1 ETH. It's still 0.
assertEq(user1.balance, 0 ether);
assertEq(address(likeRegistry).balance, 1 ether);
console.log("Balance of user1:", user1.balance);
console.log("Balance of LikeRegistry:", address(likeRegistry).balance);
}
function testBurnProfileLossOfFunds() public {
vm.deal(user1, 1 ether);
console.log("Balance of user1:", user1.balance);
console.log("Balance of LikeRegistry:", address(likeRegistry).balance);
// User1 likes user2 so LikeRegistry has 1 ETH
console.log("User1 likes User2...");
vm.prank(user1);
likeRegistry.likeUser{value: 1 ether}(user3);
console.log("Balance of user1:", user1.balance);
console.log("Balance of LikeRegistry:", address(likeRegistry).balance);
// User1 burns their profile
console.log("User1 burns their profile...");
vm.prank(user1);
profileNFT.burnProfile();
// Bug - User1 balance is not credited 1 ETH. It's still 0.
assertEq(user1.balance, 0 ether);
assertEq(address(likeRegistry).balance, 1 ether);
console.log("Balance of user1:", user1.balance);
console.log("Balance of LikeRegistry:", address(likeRegistry).balance);
}
}

Run the block user unit test with verbosity: forge test --mt testBlockProfileLossOfFunds -vvv

The unit test will pass and the following is logged to the screen:

Logs:
Balance of user1: 1000000000000000000
Balance of LikeRegistry: 0
User1 likes User2...
Balance of user1: 0
Balance of LikeRegistry: 1000000000000000000
Owner blocks user1...
Balance of user1: 0
Balance of LikeRegistry: 1000000000000000000

Run the burn user unit test with verbosity: forge test --mt testBurnProfileLossOfFunds -vvv

The unit test will pass and the following is logged to the screen:

Logs:
Balance of user1: 1000000000000000000
Balance of LikeRegistry: 0
User1 likes User2...
Balance of user1: 0
Balance of LikeRegistry: 1000000000000000000
User1 burns their profile...
Balance of user1: 0
Balance of LikeRegistry: 1000000000000000000

Recommended Mitigation: Add a refund function to LikeRegistry.sol. A user can call this function to get their user balance refunded if their profile was burned or blocked.

+ function refund() external {
+ require(profileNFT.profileToToken(msg.sender) == 0, "Must have NOT have a profile NFT");
+ require(userBalances[msg.sender] > 0, "No balance to refund");
+
+ uint256 balance = userBalances[msg.sender];
+ userBalances[msg.sender] = 0;
+
+ (bool success, ) = payable(msg.sender).call{value: balance}("");
+ require(success, "Refund failed");
+ }
Updates

Appeal created

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