T05 · Unauthorized Access and Privilege Escalation
Error
- Location
- src/skills/listNFT.ts:55
- Finding
- Marketplace receives unrestricted operator approval for all wallet-owned NFTs<![CDATA[ ## Vulnerability Details **File Location**: `src/skills/listNFT.ts:55-63` **Vulnerability Type**: Excessive ERC-721 operator approval **Risk Level**: High ### Vulnerable Code ```typescript // Check if marketplace is approved const isApproved = await nftContract.isApprovedForAll(wallet.address, process.env.MARKETPLACE_ADDRESS!); if (!isApproved) { console.log('[List] Approving marketplace...'); const approveTx = await nftContract.setApprovalForAll(process.env.MARKETPLACE_ADDRESS!, true); await approveTx.wait(); console.log('[List] Marketplace approved'); } ``` The equivalent behavior is also present in `dist/skills/listNFT.js:74-78`. ### Technical Analysis The operation is intended to list one identified NFT, but the Skill calls `setApprovalForAll`, granting the configured marketplace address permission to transfer every ERC-721 token owned by the wallet under this NFT contract. This approval also applies to tokens acquired or minted later and remains active until explicitly revoked. The ABI already contains the token-specific `approve(address,uint256)` function, but it is not used. Consequently, the implementation violates least privilege. The code also trusts `MARKETPLACE_ADDRESS` without verifying its deployed bytecode, expected contract identity, or network chain ID before signing the approval transaction. ### Attack Path 1. An attacker causes `MARKETPLACE_ADDRESS` to reference an attacker-controlled or compromised contract, such as through configuration tampering or deployment-address substitution. 2. The user invokes the Skill to list a single NFT. 3. The Skill confirms that the attacker-controlled address does not already have operator approval. 4. The wallet signs `setApprovalForAll(attackerAddress, true)`. 5. The attacker-controlled operator invokes `transferFrom` or `safeTransferFrom` against other NFTs held by the wallet. 6. The operator can continue transferring current and future NFTs until the approval is revoked. ### I ...[truncated 449 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Replace collection-wide approval with token-specific approval: ```typescript const approvedAddress = await nftContract.getApproved(tokenId); if (approvedAddress.toLowerCase() !== marketplaceAddress.toLowerCase()) { const approveTx = await nftContract.approve(marketplaceAddress, tokenId); await approveTx.wait(); } ``` 2. Add `getApproved(uint256)` to the NFT ABI and remove `setApprovalForAll` unless the user explicitly requests collection-wide authorization. 3. Validate `NFT_CONTRACT_ADDRESS` and `MARKETPLACE_ADDRESS` with `ethers.isAddress`. 4. Check the connected chain ID against an explicit allowlist before signing any transaction. 5. Use `provider.getCode(address)` to reject addresses without deployed bytecode. 6. Where possible, verify the marketplace bytecode hash or contract deployment against a trusted configuration. 7. If operator-wide approval is retained, require explicit user confirmation, clearly disclose its scope, and provide a supported revocation operation. ]]>