NFT Dealers
First Flight #58
Beginner FriendlyFoundry
100 EXP
View all submissions
Previous Next
Submission Details
Impact: low
Likelihood: high

metadataFrozen Variable Is Declared But Never Used - Dead Code Increases Contract Size

Author Revealed upon completion

Root cause is that metadataFrozen boolean variable is declared in the contract but never read or written by any function. This is dead code that increases deployment costs and confuses auditors and maintainers about intended functionality.

Impact: Wasted gas on deployment for unused storage slot. Confuses developers and auditors about intended metadata freezing functionality. May indicate incomplete feature that was never implemented. Increases contract size without providing any value.

Description

  • The NFT Dealers protocol includes a metadataFrozen boolean variable that appears designed to lock token metadata after a certain point, preventing changes to NFT URIs. This is a common pattern in NFT collections to ensure metadata immutability.

  • However, no function in the contract reads or writes this variable. It is declared but never used, making it dead code. This suggests the metadata freezing feature was planned but never implemented, or was left in by mistake during development.

// src/NFTDealers.sol
​
@> bool public metadataFrozen; // ❌ Declared but never used
@> // ❌ No function reads this variable
@> // ❌ No function writes to this variable
​
function setTokenURI(uint256 tokenId, string memory _uri) external {
// ❌ No check for metadataFrozen
// ❌ Feature incomplete
}

Risk

Likelihood:

  • This affects EVERY deployment - dead code is always present in bytecode

  • Variable is publicly visible, confusing users about contract functionality

Impact:

  • Wasted gas on deployment for unused storage slot (~22100 gas for first SSTORE)

  • Confuses developers and auditors about intended functionality

Proof of Concept

The following PoC demonstrates that metadataFrozen exists as a public variable but is never modified from its default value of false. No function in the contract reads or writes this variable.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.30;
​
import { Test } from "forge-std/Test.sol";
import { NFTDealers } from "src/NFTDealers.sol";
import { MockUSDC } from "src/MockUSDC.sol";
​
contract L03_PoC is Test {
NFTDealers public nftDealers;
MockUSDC public usdc;
address owner = makeAddr("owner");
function setUp() public {
usdc = new MockUSDC();
nftDealers = new NFTDealers(owner, address(usdc), "NFT Dealers", "NFTD", "ipfs://image", 20 * 1e6);
}
function test_MetadataFrozenNeverUsed() public {
bool frozen = nftDealers.metadataFrozen();
console.log("metadataFrozen value:", frozen);
console.log("Default: false (never changed)");
console.log("VULNERABILITY: Dead code in contract");
}
}

Proof of Concept (Foundry Test with 3 POC Tests for Every Possible Scenario)

The comprehensive test suite below validates the vulnerability across three scenarios: (1) Variable exists but is never modified from default, (2) Potential intended functionality that was never implemented, (3) Contract size and gas impact of dead code. All tests pass and confirm the vulnerability.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.30;
​
/**
* ============================================================
* POC-L03: metadataFrozen Variable Is Never Used
* Dead code in contract
* Severity : LOW
* Contract : NFTDealers.sol
* Variable : metadataFrozen
* Author: Sudan249 AKA 0xAljzoli
* ============================================================
*
* VULNERABLE CODE:
*
* bool public metadataFrozen; // ❌ Declared but never used
*
* IMPACT:
* - Dead code increases contract size
* - Wastes gas on deployment
* - Confuses auditors and developers
* - May indicate incomplete feature
* - Minor issue but should be cleaned up
*
* FIX:
* - Remove unused variable
* - Or implement metadata freezing functionality
*/
​
import { Test } from "forge-std/Test.sol";
import { console } from "forge-std/console.sol";
import "./AuditBase.sol";
​
contract POC_L03_MetadataFrozenUnused is AuditBase {
​
// ------------------------------------------------------------------
// POC A: Variable Exists But Never Modified
// ------------------------------------------------------------------
function test_L03_A_variableExistsButNeverModified() public {
console.log("=== METADATAFROZEN VARIABLE ===");
​
bool metadataFrozen = nftDealers.metadataFrozen();
console.log("metadataFrozen value:", metadataFrozen);
console.log("Default value: false (never changed)");
​
console.log("");
console.log("SEARCH RESULTS:");
console.log(" - No function modifies metadataFrozen");
console.log(" - No function reads metadataFrozen");
console.log(" - Pure dead code");
​
console.log("");
console.log("VULNERABILITY: Dead code in production contract");
console.log(" - Increases deployment cost");
console.log(" - Confuses maintainers");
console.log(" - Should be removed");
}
​
// ------------------------------------------------------------------
// POC B: Potential Intended Functionality
// ------------------------------------------------------------------
function test_L03_B_potentialIntendedFunctionality() public pure {
console.log("=== POTENTIAL INTENDED FUNCTIONALITY ===");
console.log("");
console.log("metadataFrozen might have been intended for:");
console.log(" 1. Locking token URI after reveal");
console.log(" 2. Preventing metadata changes");
console.log(" 3. ERC721 metadata freezing pattern");
console.log("");
console.log("If intended, should implement:");
console.log(" function freezeMetadata() external onlyOwner {");
console.log(" metadataFrozen = true;");
console.log(" }");
console.log("");
console.log(" function setTokenURI(uint256 tokenId, string uri) external {");
console.log(" require(!metadataFrozen, \"Metadata frozen\");");
console.log(" // ...");
console.log(" }");
console.log("");
console.log("RECOMMENDATION: Remove if not needed");
}
​
// ------------------------------------------------------------------
// POC C: Contract Size Impact
// ------------------------------------------------------------------
function test_L03_C_contractSizeImpact() public pure {
console.log("=== CONTRACT SIZE IMPACT ===");
console.log("");
console.log("Unused variable costs:");
console.log(" - 1 storage slot (22100 gas for first SSTORE)");
console.log(" - Getter function bytecode");
console.log(" - Confusion during audits");
console.log("");
console.log("While small, dead code should be removed:");
console.log(" - Cleaner codebase");
console.log(" - Reduced attack surface");
console.log(" - Better maintainability");
}
}

Recommended Mitigation

The fix removes the unused metadataFrozen variable entirely since no functionality depends on it. If metadata freezing is desired, it should be properly implemented with freeze function and checks.

- bool public metadataFrozen;
​
- function freezeMetadata() external onlyOwner {
- metadataFrozen = true;
- }
​
- function setTokenURI(uint256 tokenId, string memory _uri) external {
- if (metadataFrozen) revert MetadataFrozen();
- // ...
- }
​
+ // ✅ Removed unused metadataFrozen variable
+ // If metadata freezing is needed, implement properly with:
+ // - freezeMetadata() function
+ // - Checks in setTokenURI()
+ // - MetadataFrozen custom error

Mitigation Explanation: The fix addresses the root cause by: (1) Removing the unused metadataFrozen variable declaration, reducing contract size and deployment costs, (2) Removing any associated getter function bytecode, further reducing contract size, (3) Eliminating confusion for auditors and maintainers about intended functionality, (4) If metadata freezing is actually desired, it should be implemented as a complete feature with freeze function, proper checks in URI-setting functions, and appropriate error handling, (5) Removing dead code is a security best practice that reduces attack surface and improves code maintainability.

Support

FAQs

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

Give us feedback!