84 lines
2.4 KiB
Markdown
84 lines
2.4 KiB
Markdown
# M-03. App owner can have users' funds locked by blocking them
|
|
|
|
## Valid medium finding by someone else
|
|
|
|
## Summary
|
|
|
|
App owner can block users at will, causing users to have their funds locked.
|
|
|
|
## Vulnerability Details
|
|
|
|
`SoulboundProfileNFT::blockProfile` can block any app's user at will.
|
|
|
|
```
|
|
/// @notice App owner can block users
|
|
function blockProfile(address blockAddress) external onlyOwner {
|
|
uint256 tokenId = profileToToken[blockAddress];
|
|
require(tokenId != 0, "No profile found");
|
|
|
|
_burn(tokenId);
|
|
delete profileToToken[blockAddress];
|
|
delete _profiles[tokenId];
|
|
|
|
emit ProfileBurned(blockAddress, tokenId);
|
|
}
|
|
```
|
|
|
|
## Proof of Concept
|
|
|
|
The following code demonstrates the scenario where the app owner blocks `bob` and he is no longer able to call `LikeRegistry::likeUser`. Since the contract gives no posibility of fund withdrawal, `bob`'s funds are now locked.
|
|
|
|
Place `test_blockProfileAbuseCanCauseFundLoss` in `testSoulboundProfileNFT.t.sol`:
|
|
|
|
```
|
|
function test_blockProfileAbuseCanCauseFundLoss() public {
|
|
vm.deal(bob, 10 ether);
|
|
vm.deal(alice, 10 ether);
|
|
|
|
// mint a profile NFT for bob
|
|
vm.prank(bob);
|
|
soulboundNFT.mintProfile("Bob", 25, "ipfs://profileImage");
|
|
|
|
// mint a profile NFT for Alice
|
|
vm.prank(alice);
|
|
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
|
|
|
|
// alice <3 bob
|
|
vm.prank(alice);
|
|
likeRegistry.likeUser{value: 1 ether}(bob);
|
|
|
|
vm.startPrank(owner);
|
|
soulboundNFT.blockProfile(bob);
|
|
assertEq(soulboundNFT.profileToToken(msg.sender), 0);
|
|
|
|
vm.startPrank(bob);
|
|
vm.expectRevert("Must have a profile NFT");
|
|
// bob is no longer able to like a user, as his profile NFT is deleted
|
|
// his funds are effectively locked
|
|
likeRegistry.likeUser{value: 1 ether}(alice);
|
|
}
|
|
```
|
|
|
|
And run the test:
|
|
|
|
```
|
|
$ forge test --mt test_blockProfileAbuseCanCauseFundLoss
|
|
Ran 1 test for test/testSoulboundProfileNFT.t.sol:SoulboundProfileNFTTest
|
|
[PASS] test_blockProfileAbuseCanCauseFundLoss() (gas: 326392)
|
|
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 1.42ms (219.63µs CPU time)
|
|
|
|
Ran 1 test suite in 140.90ms (1.42ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
|
|
```
|
|
|
|
## Impact
|
|
|
|
App users can have their funds locked, as well as miss out on potential dates.
|
|
|
|
## Tools used
|
|
|
|
Manual review, tests
|
|
|
|
## Recommendations
|
|
|
|
Add a voting mechanism to prevent abuse and/or centralization of the feature.
|