cairo-auditor: add 3 missing vuln-db entries, new benchmark edge cases, and candidate classes

[omarespejel]

Summary

Audit of the cairo-auditor benchmark suite and vulnerability-db identified several gaps. This issue provides:

1. Three ready-to-commit vuln-db entries for classes only present in datasets/distilled/vuln-cards/ but missing from cairo-auditor/references/vulnerability-db/
2. Harder edge-case benchmark entries
3. Candidate new vulnerability classes mined from normalized findings

1. Missing Vuln-DB Entries

The benchmarks reference 7 vulnerability classes, but references/vulnerability-db/ only has 4. The 3 below already exist as distilled vuln-cards but need promotion to the full vuln-db format.

File: cairo-auditor/references/vulnerability-db/UNCHECKED-FEE-BOUND.md

# UNCHECKED-FEE-BOUND

## Description

Caller-provided fee/rate parameter is forwarded to deployment or storage without range validation, allowing out-of-bounds values that break protocol math.

## Vulnerable Pattern

- fee/rate parameter accepted from external caller
- passed directly to `deploy_syscall` calldata or `self.fee.write(...)` 
- no `assert(fee <= MAX)` or `assert(fee >= MIN)` guard

## Secure Pattern

- define protocol constants `MAX_FEE_BPS` / `MIN_FEE_BPS`
- assert fee within bounds before persisting or forwarding

## Detection Heuristics

- external function accepts numeric fee/rate/bps parameter
- parameter flows to `deploy_syscall`, `.write()`, or cross-contract call without intervening bound assertion
- no constant comparison (`<=`, `>=`, `<`, `>`) on the parameter in the same call path

## False Positive Caveats

- fee already validated and clamped in an immutable upstream module
- parameter is an enum/flag with bounded range by type

## Minimum Tests

- `max_fee` value succeeds
- `max_fee + 1` reverts with expected error
- zero fee handled correctly if protocol allows it

File: cairo-auditor/references/vulnerability-db/SHUTDOWN-OVERRIDE-PRECEDENCE.md

# SHUTDOWN-OVERRIDE-PRECEDENCE

## Description

When a contract combines inferred operational mode (e.g. timestamp-based shutdown) with a manually forced override mode, checking the inferred mode first allows it to shadow the admin's explicit override.

## Vulnerable Pattern

- compute inferred mode from timestamp/state
- return early when inferred mode is active
- only then check admin-set fixed override (too late)

## Secure Pattern

- read fixed/admin override first
- return fixed value when active
- evaluate inferred mode only when fixed override is not set

## Detection Heuristics

- function returns a mode/status derived from two sources (inferred + explicit)
- inferred path has early return before explicit/fixed path is checked
- both paths compare against a sentinel (e.g. `!= NONE`)

## False Positive Caveats

- fixed override feature explicitly disabled by design (documented)
- no emergency/admin override semantics in protocol requirements
- single-source mode determination (no dual path)

## Minimum Tests

- set both inferred mode and fixed override active; assert returned mode equals fixed override
- fixed override alone returns correct value
- inferred mode alone returns correct value

File: cairo-auditor/references/vulnerability-db/SYSCALL-SELECTOR-FALLBACK-ASSUMPTION.md

# SYSCALL-SELECTOR-FALLBACK-ASSUMPTION

## Description

Helper function issues `call_contract_syscall` with one selector, then on error retries with an alternate selector (e.g. camelCase vs snake_case). In modern Starknet execution semantics, a failed external call reverts the transaction scope, making the fallback branch dead code that masks real errors.

## Vulnerable Pattern

- `call_contract_syscall(token, SELECTOR_A, calldata)` 
- `if result.is_err()` branch retries with `SELECTOR_B`
- assumes failed syscall is recoverable in same transaction

## Secure Pattern

- use one canonical selector and fail hard on syscall error
- `.unwrap_syscall()` directly without fallback branching

## Detection Heuristics

- `call_contract_syscall` followed by `.is_err()` check
- error branch contains another `call_contract_syscall` with different selector constant
- both selectors target the same logical function (e.g. `balanceOf` / `balance_of`)

## False Positive Caveats

- offchain simulation helper code outside onchain execution context
- explicitly documented compatibility wrapper for non-reverting environments
- view-only utility used exclusively in tests

## Minimum Tests

- force failing syscall; assert function reverts without retry behavior
- successful call with canonical selector returns expected value

2. Harder Edge-Case Benchmark Entries

The current benchmarks pair each class with a clear vuln + clear safe case. These edge cases test partial mitigations and subtle variants:

Proposed new cases for cairo_auditor_benchmark.jsonl:

{"case_id": "aa_self_call_partial_guard_01", "class_id": "AA-SELF-CALL-SESSION", "expected_detect": true, "source": "Session-key path blocks admin selectors but not self-call target", "source_url": "", "code": "fn __execute__(ref self: ContractState, calls: Array<Call>) -> Array<Span<felt252>> {\n    let session_key = *get_tx_info().unbox().signature.at(0);\n    let policy = self.session_keys.get_policy(session_key);\n    for call in calls.span() {\n        assert(!is_admin_selector(*call.selector), 'blocked');\n    };\n    execute_calls(calls.span())\n}"}
{"case_id": "fee_bound_partial_01", "class_id": "UNCHECKED_FEE_BOUND", "expected_detect": true, "source": "Fee checked against zero but no upper bound", "source_url": "", "code": "fn set_fee(ref self: ContractState, new_fee: u16) {\n    assert(new_fee > 0, 'FEE_ZERO');\n    self.swap_fee.write(new_fee);\n}"}
{"case_id": "upgrade_timelock_zero_delay_01", "class_id": "IMMEDIATE_UPGRADE_WITHOUT_TIMELOCK", "expected_detect": true, "source": "Timelock pattern with configurable delay that can be set to zero", "source_url": "", "code": "fn schedule_upgrade(ref self: ContractState, new_class_hash: ClassHash) {\n    self.pending_upgrade.write(new_class_hash);\n    self.upgrade_scheduled_at.write(get_block_timestamp());\n}\nfn execute_upgrade(ref self: ContractState) {\n    let delay = self.upgrade_delay.read();\n    assert(get_block_timestamp() >= self.upgrade_scheduled_at.read() + delay, 'too early');\n    replace_class_syscall(self.pending_upgrade.read()).unwrap_syscall();\n}\nfn set_delay(ref self: ContractState, new_delay: u64) {\n    assert(get_caller_address() == self.admin.read(), 'not admin');\n    self.upgrade_delay.write(new_delay);\n}"}
{"case_id": "critical_addr_partial_guard_01", "class_id": "CRITICAL_ADDRESS_INIT_WITHOUT_NONZERO_GUARD", "expected_detect": true, "source": "Constructor validates admin but not other critical addresses", "source_url": "", "code": "fn constructor(ref self: ContractState, admin: ContractAddress, oracle: ContractAddress, vault: ContractAddress) {\n    assert(admin.is_non_zero(), 'admin zero');\n    self.admin.write(admin);\n    self.oracle.write(oracle);\n    self.vault.write(vault);\n}"}

3. Candidate New Vulnerability Classes

Mined from normalized findings that are NOT yet in vuln-db or vuln-cards:

Candidate Class ID	Source Finding	Description
UNBOUNDED_SHORT_STRING_JOIN	ERIM-NOSTRA-L01	Token symbol composition via join_short_strings without length validation causes revert on overlong symbols
REDUNDANT_ZERO_STORAGE_WRITE	ERIM-NOSTRA-BP02	Constructor writes default zero/false values already guaranteed by Cairo storage semantics, wasting gas
STALE_DEPENDENCY_VERSION	ERIM-NOSTRA-I03	Scarb.toml pins outdated starknet version that drifts from peer dependency expectations
EXTENSION_PARITY_MISMATCH	CSC-VESU-002	Protocol extension implementation omits capability present in sibling extensions, creating inconsistent behavior across pools
MISSING_CALLER_VERIFICATION_ON_CALLBACK	KAPAN-005 (inferred)	Flash loan callback (on_flash_loan) lacks caller verification, allowing unauthorized invocations

Recommendation: prioritize UNBOUNDED_SHORT_STRING_JOIN and MISSING_CALLER_VERIFICATION_ON_CALLBACK as they are security-relevant and generalizable.

4. Benchmark Diversity Observation

Core and realworld benchmark suites are nearly identical (same code snippets with minor context variations). To improve eval quality:

• Realworld suite should use actual deployed contract code from repos like ForgeYields, Medialane, Kiroshi
• Consider adding negative cases from OZ cairo-contracts for broader false-positive coverage
• The rw_aa_self_call_safe_agent_account_01 case has a subtle gap: session-key path calls execute_calls() which doesn't block self-targeting calls; safety relies solely on allowed_contract policy check

[omarespejel]

Status update from PR #9 (codex/audit-rule-calibration-v021): substantial portions of this issue are now implemented.

Implemented:

• Added missing vuln-db entries:
  • UNCHECKED-FEE-BOUND.md
  • SHUTDOWN-OVERRIDE-PRECEDENCE.md
  • SYSCALL-SELECTOR-FALLBACK-ASSUMPTION.md
• Added vuln-db parity for new class:
  • CONSTRUCTOR-DEAD-PARAM.md
• Added wave-2 classes + docs:
  • CEI-VIOLATION-ERC1155.md
  • FEES-RECIPIENT-ZERO-DOS.md
  • NO-ACCESS-CONTROL-MUTATION.md
• Expanded benchmark suites from 20 to 26 cases each.
• Added reproducible external scan runner:
  • scripts/quality/scan_external_repos.py
• Ran second external trial on a fresh 7-repo set with artifacts in evals/reports/data/external-repo-scan-wave2-2026-03-09.* and markdown summary in evals/reports/external-repo-scan-wave2-2026-03-09.md.

Still open / follow-up to keep here:

• Add stricter CI parity gate that enforces: every benchmark class_id must map to a vuln-db file.
• Add harder edge cases for partial mitigations (especially access-control and reentrancy guard variants).
• Manual TP/FP labeling pass for the new wave-2 external scan before promoting to release trend metrics.