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 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

評価

難易度
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 はありません

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

fulldecent/erc721 のほかの issue

fulldecent/erc721 の issue をすべて見る

似ている issue

Blockchain の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。