Submission Details
Severity:
`onlyOwner` modifier in `SoulboundProfileNFT::blockProfile` causes centralization risk and can block users funds inside the `LikeRegistry` contract.
**Description:** The `blockProfile` function in `SoulboundProfileNFT` uses the `onlyOwner` modifier, which restricts the ability to block a profile to the contract owner. This creates a centralization risk as only the owner can block a profile. This also means that any user blocked won't be able to like or get liked by other users. Any funds sent to the `LikeRegistry` contract by the blocked user will be lost.
**Impact:** Anyone can be blocked if the owner has decided to do so, and the blocked user will lose any funds sent to the `LikeRegistry` contract.
**Proof of Concept:**
Add the following code at the end of `testSoulboundProfileNFT.t.sol` :
```javascript
function testAnyoneCanBeBlockedAndLoseFunds() public {
LikeRegistry likeRegistry;
assertEq(address(likeRegistry).balance, 0);
// mint profiles
// the first one
address firstLover = makeAddr("firstLover");
vm.prank(firstLover);
soulboundNFT.mintProfile("Alice", 75, "ipfs://aliceProfileImage");
// the second one
address secondLover = makeAddr("secondLover");
vm.prank(secondLover);
soulboundNFT.mintProfile("Bob", 25, "ipfs://bobProfileImage");
// make them like each other
likeRegistry = new LikeRegistry(address(soulboundNFT));
// the first one
vm.deal(firstLover, 1 ether);
uint256 firstLoverStartingBalance = address(firstLover).balance;
vm.prank(firstLover);
likeRegistry.likeUser{value: 1 ether}(secondLover);
// owner decides to block first lover
vm.prank(owner);
soulboundNFT.blockProfile(firstLover);
// check that newTokenId is 0
uint256 newTokenId = soulboundNFT.profileToToken(firstLover);
assertEq(newTokenId, 0);
// assert the second lover cannot like the first lover anymore
vm.deal(secondLover, 1 ether);
vm.prank(secondLover);
vm.expectRevert("Liked user must have a profile NFT");
likeRegistry.likeUser{value: 1 ether}(firstLover);
// check that the balance of the contract is 1 ether and balance of user has decreased by 1 ether
assertEq(address(likeRegistry).balance, 1 ether);
assertEq(
address(firstLover).balance,
firstLoverStartingBalance - 1 ether
);
}
```
**Recommended Mitigation:**
- Allow the blocked user to withdraw any ETH sent to the `LikeRegistry` and that has not yet been sent to a MultiSig wallet.
```diff
contract LikeRegistry is Ownable {
...
+ function withdrawIfBlocked() external {
+ require(
+ profileNFT.profileToToken(msg.sender) == 0,
+ "Profile not blocked"
+ );
+ uint256 balance = userBalances[msg.sender];
+ require(balance > 0, "No balance to withdraw");
+ userBalances[msg.sender] = 0;
+ (bool success, ) = payable(msg.sender).call{value: balance}("");
+ require(success, "Transfer failed");
+ }
}
```
Updates
Appeal created
n0kto 7 months ago
Submission Judgement Published 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
Rewards breakdown
nSLOC
197This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.