121 lines
4.5 KiB
Markdown
121 lines
4.5 KiB
Markdown
# Blocked User Can Call `SoulboundProfileNFT::mintProfile` Again
|
|
|
|
## A valid finding but the actual severity is Medium instead of High. written by myself.
|
|
|
|
## Summary
|
|
|
|
Due to missing blocked profile handling, any blocked profile can call `SoulboundProfileNFT::mintProfile` again to mint and take part on the system. The purpose of blocking a profile no longer valid, since they could mint a new profile and continue interacting with existing `LikeRegistry` and `MultiSig`.
|
|
|
|
## Vulnerability Details
|
|
|
|
When `SoulboundProfileNFT::blockProfile` is called by the owner, it will burn the profile token, delete onchain variable related to the burnt tokenId and delete the profileToToken of the address.
|
|
|
|
```solidity
|
|
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);
|
|
}
|
|
```
|
|
|
|
So after the `SoulboundNFT::blockProfile` being called, no one can call `LikeRegistry::likeUser` to the blocked address.
|
|
|
|
```solidity
|
|
function likeUser(address liked) external payable {
|
|
[...]
|
|
|
|
require(profileNFT.profileToToken(msg.sender) != 0, "Must have a profile NFT");
|
|
require(profileNFT.profileToToken(liked) != 0, "Liked user must have a profile NFT");
|
|
|
|
[...]
|
|
}
|
|
```
|
|
|
|
The `LikeRegistry::likeUser` require both the caller to have existing profile tokens. So blocked user unable to like or being liked.
|
|
|
|
But the `SoulboundProfileNFT::mintProfile` doesn't have any check if the user or profile is blocked or not. So if someone has already blocked, they can call the mint function again.
|
|
|
|
```solidity
|
|
function mintProfile(string memory name, uint8 age, string memory profileImage) external {
|
|
require(profileToToken[msg.sender] == 0, "Profile already exists");
|
|
|
|
uint256 tokenId = ++_nextTokenId;
|
|
_safeMint(msg.sender, tokenId);
|
|
|
|
// Store metadata on-chain
|
|
_profiles[tokenId] = Profile(name, age, profileImage);
|
|
profileToToken[msg.sender] = tokenId;
|
|
|
|
emit ProfileMinted(msg.sender, tokenId, name, age, profileImage);
|
|
}
|
|
```
|
|
|
|
There is no check if sender has its profile blocked or not previously.
|
|
|
|
## POC
|
|
|
|
```solidity
|
|
function testBlockedProfileMintAgain() public {
|
|
vm.prank(user);
|
|
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
|
|
|
|
uint256 tokenId = soulboundNFT.profileToToken(user);
|
|
assertEq(tokenId, 1, "Token should exist before blocking");
|
|
|
|
vm.prank(owner);
|
|
soulboundNFT.blockProfile(user);
|
|
|
|
uint256 newTokenId = soulboundNFT.profileToToken(user);
|
|
assertEq(newTokenId, 0, "Token should be removed after blocking");
|
|
|
|
vm.prank(user);
|
|
vm.expectRevert();
|
|
// blocked profile should not be able to mint from the same wallet address again
|
|
soulboundNFT.mintProfile("Alice", 25, "ipfs://profileImage");
|
|
}
|
|
```
|
|
|
|
## Impact
|
|
|
|
- Any blocked profile can mint again and continue their activity at this protocol
|
|
- Blocked profile can mint again and mint a profile from another wallet, then make the other profile to match with his blocked account, and withdraw all of their own fund.
|
|
|
|
## Recommendations
|
|
|
|
Add new storage variable to check if a wallet address is blocked or not.
|
|
|
|
```diff
|
|
--- a/src/SoulboundProfileNFT.sol
|
|
+++ b/src/SoulboundProfileNFT.sol
|
|
@@ -20,6 +20,7 @@ contract SoulboundProfileNFT is ERC721, Ownable {
|
|
|
|
mapping(address => uint256) public profileToToken; // Maps user to their profile NFT
|
|
mapping(uint256 => Profile) private _profiles; // Stores profile metadata
|
|
+ mapping(address => bool) public profileBlocked;
|
|
|
|
event ProfileMinted(address indexed user, uint256 tokenId, string name, uint8 age, string profileImage);
|
|
event ProfileBurned(address indexed user, uint256 tokenId);
|
|
@@ -29,6 +30,7 @@ contract SoulboundProfileNFT is ERC721, Ownable {
|
|
/// @notice Mint a soulbound NFT representing the user's profile.
|
|
function mintProfile(string memory name, uint8 age, string memory profileImage) external {
|
|
require(profileToToken[msg.sender] == 0, "Profile already exists");
|
|
+ require(profileBlocked[msg.sender] == false, "Profile is blocked");
|
|
|
|
uint256 tokenId = ++_nextTokenId;
|
|
_safeMint(msg.sender, tokenId);
|
|
@@ -61,6 +63,7 @@ contract SoulboundProfileNFT is ERC721, Ownable {
|
|
_burn(tokenId);
|
|
delete profileToToken[blockAddress];
|
|
delete _profiles[tokenId];
|
|
+ profileBlocked[blockAddress] = true;
|
|
|
|
emit ProfileBurned(blockAddress, tokenId);
|
|
}
|
|
```
|
|
|