Bug Bounty Report: phantom token IDs revert with authorization codes (003003/003004) instead of NOT_VALID_NFT (003002) on transferFrom/approve/safeTransferFrom

未关闭
  • 哪里不对？
#300 2 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

还没有人认领这个 Issue。

评估

难度
2/5
预计耗时
1-3 小时
新手友好度
55/100
Issue 类型
缺陷
描述清晰度
描述清楚
活跃度
活跃
技术栈
solidity
领域
blockchain

调研方向

Start in src/contracts/tokens/nf-token.sol: compare the modifier order on transferFrom (around lines 174-175), approve (197-198) and _safeTransferFrom (431-432), then read canTransfer and canOperate (64-88) and validNFToken (99). Reproduce with the Foundry test in the report, which calls ownerOf, transferFrom, approve and safeTransferFrom on phantom ID 999. Done means all entry points return 003002 for a nonexistent token, or the natspec documents the per-entry-point codes. The maintainer must choose between those two, so confirm the direction in the thread before editing.

由索引模型根据 Issue 内容生成。

描述

Found while reviewing src/contracts under the bug bounty program (latest release, @nibbstack/erc721 2.6.1).

What

transferFrom, approve, and _safeTransferFrom declare their authorization modifier before validNFToken, so for a token ID that does not exist the transaction reverts with an authorization-flavored code instead of NOT_VALID_NFT:

src/contracts/tokens/nf-token.sol:174-175 (transferFrom):

    canTransfer(_tokenId)
    validNFToken(_tokenId)

src/contracts/tokens/nf-token.sol:197-198 (approve):

    canOperate(_tokenId)
    validNFToken(_tokenId)

src/contracts/tokens/nf-token.sol:431-432 (_safeTransferFrom): same order.

Modifiers run in declaration order, and both canTransfer and canOperate read idToOwner[_tokenId] directly:

src/contracts/tokens/nf-token.sol:64-69 and :82-88:

address tokenOwner = idToOwner[_tokenId];
require(
  tokenOwner == msg.sender || ownerToOperators[tokenOwner][msg.sender],
  NOT_OWNER_OR_OPERATOR        // "003003"
);

For a phantom ID tokenOwner is address(0), which never equals msg.sender, so the require fails with 003003 (approve) or 003004 (transferFrom) and validNFToken (nf-token.sol:99, NOT_VALID_NFT = "003002") is never reached.

Meanwhile the same natspec promises the validity check: src/contracts/tokens/nf-token.sol:160-161 says "Throws if _tokenId is not a valid NFT", and ownerOf (:258) plus getApproved (:273) do return 003002 for the same phantom ID.

Impact

The revert code for "this token does not exist" depends on which entry point the caller hit: 003002 from ownerOf/getApproved, but 003003/003004 from transferFrom/approve/safeTransferFrom. Indexers, front ends, and integrators that branch on 003002 to distinguish "nonexistent token" from "not authorized" misclassify every nonexistent-token call that arrives through a transfer or approve entry point, and the documented behavior does not mention this. This falls under "any undocumented and unintuitive behavior" in the program scope.

Suggested fix

List validNFToken(_tokenId) before canTransfer/canOperate on all three entry points (Solidity checks modifiers in order), or document the actual codes per entry point in the natspec.

Proof (Foundry, solc 0.8.26, forge-std)

[PASS] test_approve_phantom_returns_003003_not_003002() (gas: 17523)
[PASS] test_ownerOf_phantom_returns_003002() (gas: 12216)
[PASS] test_safeTransferFrom_phantom_returns_003004_not_003002() (gas: 20329)
[PASS] test_transferFrom_phantom_returns_003004_not_003002() (gas: 20225)
Suite result: ok. 4 passed; 0 failed; 0 skipped

Test contract:

contract RevertCodeMock is NFToken, Ownable {
  function mint(address _to, uint256 _tokenId) external onlyOwner { super._mint(_to, _tokenId); }
}
// setUp: nft.mint(alice, 1); phantom id = 999
vm.expectRevert(bytes("003002")); nft.ownerOf(999);
vm.prank(alice); vm.expectRevert(bytes("003004")); nft.transferFrom(alice, alice, 999);
vm.prank(alice); vm.expectRevert(bytes("003003")); nft.approve(alice, 999);
vm.prank(alice); vm.expectRevert(bytes("003004")); nft.safeTransferFrom(alice, alice, 999);

Tested against tag 2.6.1 (commit 198dec9).

主要语言
Solidity
星标
1k
派生
391
30 天内没有已合并 PR

环境准备

  • 没有 Dockerfile 或 Docker Compose 文件
  • 没有 Pull Request 模板
  • 阅读贡献指南

从这里开始

  1. 先读完整个 Issue，再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库，在一个分支上完成修改。
  4. 提交 Pull Request，并在描述里引用这个 Issue 编号。

fulldecent/erc721 的其他 Issue

查看 fulldecent/erc721 的全部 Issue

相似的 Issue

更多 Blockchain Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。