DatingDapp

AI First Flight #6
Beginner FriendlyFoundrySolidityNFT
EXP
View results
Submission Details
Impact: low
Likelihood: medium
Invalid

[L-04] Soulbound NFT does not override `approve()` and `setApprovalForAll()`, allowing useless approvals

Description

SoulboundProfileNFT overrides transferFrom() and safeTransferFrom() to revert, but approve() and setApprovalForAll() are inherited from ERC721 without override. Users can set approvals that can never be exercised.

Vulnerability Details

// src/SoulboundProfileNFT.sol, line 69-77
function transferFrom(address, address, uint256) public pure override {
revert SoulboundTokenCannotBeTransferred(); // @> blocked
}
function safeTransferFrom(address, address, uint256, bytes memory) public pure override {
revert SoulboundTokenCannotBeTransferred(); // @> blocked
}
// @> approve() — NOT overridden, inherited from ERC721
// @> setApprovalForAll() — NOT overridden, inherited from ERC721

Calling approve(spender, tokenId) and setApprovalForAll(operator, true) both succeed and emit Approval/ApprovalForAll events. The approved spender can never actually transfer the token because transferFrom and safeTransferFrom always revert. The on-chain approval state is misleading — it suggests the token is tradeable when it is not.

Risk

Likelihood:

  • Any token holder can call approve() or setApprovalForAll(). Marketplace integrations or wallets that auto-approve operators (like OpenSea's Seaport) will call these functions as part of their standard flow.

Impact:

  • Users waste gas on approvals that can never be exercised. Indexers and marketplaces tracking approvals show the token as "approved for transfer" when it is non-transferable, creating misleading on-chain state that conflicts with the soulbound design.

PoC

The test shows that both approve() and setApprovalForAll() succeed and update on-chain state, but the approved operator still cannot transfer. The approval is set but useless — gas is wasted and the state is misleading.

function testApproveSucceedsOnSoulbound() public {
vm.prank(alice);
profileNFT.mintProfile("Alice", 25, "ipfs://alice");
uint256 tokenId = profileNFT.profileToToken(alice);
// approve succeeds — should revert for soulbound tokens
vm.prank(alice);
profileNFT.approve(bob, tokenId);
assertEq(profileNFT.getApproved(tokenId), bob); // approval state set
// setApprovalForAll also succeeds
vm.prank(alice);
profileNFT.setApprovalForAll(bob, true);
assertTrue(profileNFT.isApprovedForAll(alice, bob)); // operator approved
// But bob can't actually do anything with the approval
vm.prank(bob);
vm.expectRevert(SoulboundProfileNFT.SoulboundTokenCannotBeTransferred.selector);
profileNFT.transferFrom(alice, bob, tokenId);
}

Recommendations

Override both functions to revert with the same error used for transfers. This makes the soulbound restriction complete — not just blocking transfers but also blocking the approval step, so users get immediate feedback that the token is non-transferable.

+ function approve(address, uint256) public pure override {
+ revert SoulboundTokenCannotBeTransferred();
+ }
+
+ function setApprovalForAll(address, bool) public pure override {
+ revert SoulboundTokenCannotBeTransferred();
+ }
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 16 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!