Bid Beasts
First Flight #49
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: low
Invalid

Non-immutable `BBERC721` reference may allow reassignment

ilhamitugral

Root + Impact

Description

  • Normal behavior: The contract should permanently reference a single BidBeasts NFT contract address that is assigned during deployment and cannot be changed afterwards. Callers and users expect that marketplace logic always interacts with the same NFT contract address provided at deployment.

  • Specific issue: The BidBeastsNFTMarket::BBERC721 field is declared as a non-immutable state variable and is only assigned in the constructor. Although the current source code contains no setter, marking the variable non-immutable leaves room for accidental or intentional future reassignment (for example by adding a setter, introducing an upgrade that modifies storage, or by incorrectly-written proxy/delegatecall logic). This weakens the guarantee that the marketplace will always point to the original NFT contract and increases the attack surface and chance of human error.

// Root cause in the codebase with @> marks to highlight the relevant section
contract BidBeastsNFTMarketPlace {
// @> vulnerable: not immutable, writable storage slot
BidBeasts public BBERC721;
​
constructor(address _BidBeastsNFT) {
// @> assigned only at construction but not enforced by compiler as immutable
BBERC721 = BidBeasts(_BidBeastsNFT);
}
​
// ... rest of contract ...
}

Risk

Likelihood:

  • Reassignment happens during future maintenance or upgrades when a developer adds a setter or modifies initialization logic without realizing the security implication.

  • Reassignment occurs during an upgrade/proxy flow or via a badly-written delegatecall path that unintentionally writes to this storage slot.

Impact:

  • Marketplace operations (e.g., mint, transferFrom, buy, etc.) may be redirected to a different, attacker-controlled or unintended NFT contract — causing users to interact with incorrect token logic, lose funds, or receive unexpected tokens.

  • Trust assumptions are broken: users may mint or trade NFTs under the assumption they interact with Collection A while the marketplace actually points to Collection B, enabling confusion, scams, or economic damage to users and reputation damage to the project.

Proof of Concept

// PoC (defensive demonstration)
// 1) Current code is non-immutable and allows reassignment if a setter exists.
// This PoC shows a minimal setter that WOULD compile against the current declaration
// (this is a defensive demonstration, not an exploit script).
​
contract DemoMutableMarket {
// matches the repository pattern: writable storage
BidBeasts public BBERC721;
​
constructor(address _BidBeastsNFT) {
BBERC721 = BidBeasts(_BidBeastsNFT);
}
​
// Hypothetical setter — this compiles and allows reassignment
function setBBERC721(address _new) external /*normally onlyOwner*/ {
BBERC721 = BidBeasts(_new); // writable: reassignment succeeds under current declaration
}
}
​
// 2) Compare with immutable declaration — the same setter will NOT compile.
// Replacing the declaration with `immutable` causes a compiler error on reassignment.
​
contract DemoImmutableMarket {
// immutable enforces single assignment at construction time
BidBeasts public immutable BBERC721;
​
constructor(address _BidBeastsNFT) {
BBERC721 = BidBeasts(_BidBeastsNFT); // allowed: constructor assignment
}
​
// The following function would fail to compile — demonstrate mitigation at compile-time:
/*
function setBBERC721(address _new) external {
BBERC721 = BidBeasts(_new); // ❌ compile-time error: "Cannot assign to immutable variable"
}
*/
}

Note: The first demo shows that given a writable declaration, a setter could be added and would compile; the second demo shows that marking the variable immutable moves the enforcement into the compiler and prevents future reassignment at compile time.

Recommended Mitigation

- BidBeasts public BBERC721;
+ BidBeasts public immutable BBERC721;
​
constructor(address _BidBeastsNFT) {
BBERC721 = BidBeasts(_BidBeastsNFT);
}

Additional recommendations (defense-in-depth):

  • If the design requires the NFT address to be changeable in the future, introduce a controlled, auditable pattern instead:

    • Provide a restricted setter protected by a robust access control (e.g., onlyOwner where owner is a multisig) and require emitting an event on every update.

    • Combine with a timelock or multisig approval flow to prevent immediate unilateral reassignments.

  • Add unit tests that assert the NFT address is immutable (or that setter is restricted) and add fuzzing/invariant tests to catch accidental writes to that storage slot.

  • During audits, include checks for delegatecall, upgradeability proxies, and any patterns that could write to arbitrary storage slots; ensure storage layouts are well-documented to prevent accidental overwrites after upgrades.

Updates

Lead Judging Commences

cryptoghost Lead Judge about 1 month ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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