SNARKeling Treasure Hunt

First Flight #59
Beginner FriendlyGameFiFoundry
100 EXP
Submission Details
Impact: low
Likelihood: low

Missing naming conventions for immutable and storage variables

Author Revealed upon completion

Missing naming conventions for immutable and storage variables

Description

  • Consider using an i_ prefix for immutable variables and s_ for storage variables to make their roles immediately distinguishable.

  • The direct consequence of missing i_ on _treasureHash is documented in a high vulnerability of a replay attack. The visual similarity between _treasureHash (immutable) and treasureHash (calldata parameter) made the following key mismatch easy to miss. Had the immutable been named i_treasureHash, the divergence between claimed[i_treasureHash] and claimed[treasureHash] would have been immediately visible to any reviewer, and likely caught before deployment.

// @> immutables — should be i_
IVerifier private verifier;
address private immutable owner;
bytes32 private immutable _treasureHash;
// @> storage — should be s_
mapping(bytes32 => bool) public claimed;
uint256 public claimsCount;
bool private paused;
bool private locked;
// check reads immutable — wrong key, never true
if (claimed[_treasureHash]) revert AlreadyClaimed(treasureHash); // @> _treasureHash vs treasureHash visually ambiguous
// write targets calldata arg — different key entirely
claimed[treasureHash] = true; // @> mismatch made harder to catch without i_ prefix

Risk

Likelihood:

  • The absence of naming conventions affects every immutable and storage variable in the contract, meaning any reviewer or future developer working across the codebase will encounter this ambiguity consistently.

  • The visual similarity between _treasureHash (immutable) and treasureHash (calldata parameter) directly contributed to the key mismatch documented in the high-severity replay vulnerability, making this pattern likely to cause or mask future bugs during modifications.

Impact:

  • Missing prefixes reduce immediate distinguishability between immutable and storage variables, increasing the cognitive burden on reviewers and the risk of subtle bugs in future modifications.

Proof of Concept

// immutables — should be i_
IVerifier private verifier;
address private immutable owner;
bytes32 private immutable _treasureHash;
// storage — should be s_
mapping(bytes32 => bool) public claimed;
uint256 public claimsCount;
bool private paused;
bool private locked;

Recommended Mitigation

- address private immutable owner;
- bytes32 private immutable _treasureHash;
+ address private immutable i_owner;
+ bytes32 private immutable i_treasureHash;
- mapping(bytes32 => bool) public claimed;
- uint256 public claimsCount;
- bool private paused;
- bool private locked;
+ mapping(bytes32 => bool) public s_claimed;
+ uint256 public s_claimsCount;
+ bool private s_paused;
+ bool private s_locked;

Support

FAQs

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

Give us feedback!