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

Broken Soulbound token if token is owned by a contract

f3d0ss

Summary

Even though the soulbound token functionality explicitly prevents transfers between externally owned accounts, a vulnerability exists when the NFT is owned by a contract. In such cases, the contract's administrative logic can be exploited to change the contract's owner, effectively breaking the soulbound property.

Vulnerability Details

The contract enforces the non-transferable nature of the NFT by reverting any transfer attempt using transferFrom and safeTransferFrom. However, if the token is owned by a contract (rather than a standard externally owned account), that contract might have its own methods or administrative privileges that allow the owner of the contract to change state, including the ownership of the contract. As a result, an NFT that is supposed to be soulbound can have its owner changed indirectly through the administrative functions of the contract that holds it.

Impact

  • The core concept of a soulbound token "that the token is permanently bound to a specific identity" can be undermined if the NFT's owner can be changed by the owning contract.

  • If an NFT is held by a contract with administrative functionality, a potentially malicious or compromised administrator could reassign the contract owning the token, thereby breaking the intended non-transferable (soulbound) characteristic.

POC

In the foundry test file add the following test:

function testTransferNFTThroughContractOwner() public {
vm.startPrank(user);
SCWallet scWallet = new SCWallet();
scWallet.execute(
address(soulboundNFT),
0,
abi.encodeCall(SoulboundProfileNFT.mintProfile, ("Alice", 18, "ipfs://profileImageAlice"))
);
scWallet.transferOwnership(user2);
vm.stopPrank();
// Now user2, as the new owner, can perform a privileged action on the soulbound NFT
vm.prank(user2);
scWallet.execute(address(soulboundNFT), 0, abi.encodeCall(SoulboundProfileNFT.burnProfile, ()));
}

And the following contract:

import "@openzeppelin/contracts/access/Ownable.sol";
import "@openzeppelin/contracts/interfaces/IERC721Receiver.sol";
​
contract SCWallet is Ownable, IERC721Receiver {
constructor() Ownable(msg.sender) {}
// Execute an arbitrary call
// "to" is the target address, "value" the amount of Ether to send and "data" is the calldata.
​
function execute(address to, uint256 value, bytes calldata data) external onlyOwner returns (bytes memory) {
(bool success, bytes memory result) = to.call{value: value}(data);
require(success, "Transaction failed");
return result;
}
​
// Allow contract to receive Ether
receive() external payable {}
​
// Required for receiving ERC721 tokens.
function onERC721Received(address, address, uint256, bytes calldata) pure external returns (bytes4) {
return IERC721Receiver.onERC721Received.selector;
}
}

Tools Used

  • Manual Code Inspection: Reviewed the NFT contract code to confirm that transfer functions revert, while also examining how contracts owning NFTs might have alternative mechanisms to change ownership.

  • Foundry: Simulated scenarios where NFTs were minted to and held by contracts with administrative functions to demonstrate potential ownership reassignment.

Recommendations

  • Implement a check during the minting process that prevents tokens from being minted to contracts, enforcing that only externally owned accounts (EOAs) can receive NFTs.

Updates

Appeal created

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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