Skip to content

Fix decode_instructions panics on malformed inputs#205

Closed
Copilot wants to merge 19 commits into
shahankhatch/add-fuzz-targetsfrom
copilot/sub-pr-204
Closed

Fix decode_instructions panics on malformed inputs#205
Copilot wants to merge 19 commits into
shahankhatch/add-fuzz-targetsfrom
copilot/sub-pr-204

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 15, 2026

Fuzz targets were crashing (exit code 77) because decode_instructions had four panic sites reachable with malformed transaction data.

Root causes

  • account_keys[ci.program_id_index as usize] — unbounded index into account keys array
  • account_keys[i as usize] — same for per-instruction account indices
  • account_keys[0] — panics on empty account keys
  • panic!("No visualizer available...") — explicit panic when no visualizer matches

Changes

  • src/core/instructions.rs: Replace map() + bare indexing with filter_map() that skips instructions whose program_id_index or account indices are out of bounds — mirrors the already-safe decode_v0_instructions path. Use .first() with a default-pubkey fallback for the sender key. Replace the explicit panic! with ok_or_else(|| VisualSignError::ParseError(...)).
// Before: panics on malformed input
let instructions: Vec<Instruction> = message.instructions.iter().map(|ci| Instruction {
    program_id: account_keys[ci.program_id_index as usize],  // 💥
    ...
}).collect();

// After: skips invalid instructions gracefully
let instructions: Vec<Instruction> = message.instructions.iter().filter_map(|ci| {
    let idx = ci.program_id_index as usize;
    if idx >= account_keys.len() { return None; }
    ...
}).collect();

📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

Property tests verify that decode_idl_data and parse_instruction_with_idl
never panic regardless of IDL shape or instruction byte content, and that
valid borsh-encoded bytes always round-trip through the parser cleanly.

- fuzz_idl_parsing.rs: crash-safety tests covering random IDLs, valid
  discriminator prefixes with random arg bytes, defined struct types,
  nested Vec/Option containers, and SizeGuard boundary conditions
- pipeline_integration.rs: end-to-end tests through parse_transaction
  covering discriminator dispatch, field count invariants, named accounts,
  and multi-program transactions

Depends on solana_parser::arb (solana-parser-add-arbitrary branch) which
provides prop_recursive-based strategies for IdlType and correlated
borsh byte generators for roundtrip assertions.

Adds src/.cargo/config.toml (gitignored) as local dev override to redirect
solana_parser git dep to local checkout without committing a [patch] block.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two libFuzzer targets covering the full visualsign-solana stack:
- fuzz_transaction_string: arbitrary bytes into transaction_string_to_visual_sign
- fuzz_versioned_transaction: arbitrary bytes deserialized as VersionedTransaction
  then passed to versioned_transaction_to_visual_sign

Run with: cargo +nightly fuzz run <target>
(from src/chain_parsers/visualsign-solana/fuzz/)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- proptest label: runs cargo test -p visualsign-solana
- fuzz label: installs nightly + cargo-fuzz, runs each fuzz target for 30s
- ubuntu job: restricted to main push/PR to avoid triggering on label events

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Runs cargo fmt to fix formatting in test files. Removes clippy steps
from proptest and fuzz jobs — already enforced by the ubuntu job.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
On crash, extracts the libFuzzer summary (everything after the ─── line)
and posts it as a PR comment via gh. No artifacts or separate jobs needed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move proptest and fuzz jobs out of main.yml into dedicated workflow
files (proptest.yml, fuzz.yml) so they appear as distinct named checks.
Add pull-requests: write permission to fuzz job to allow posting crash
comments via gh pr comment.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Shared composite action posts crash/failure output as a PR comment
and tags @copilot to fix the issue. Fuzz and proptest workflows use
it via extract steps that write output to GITHUB_OUTPUT.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: shahan-khatchadourian-anchorage <263420032+shahan-khatchadourian-anchorage@users.noreply.github.com>
Copilot AI changed the title [WIP] Add cargo fuzz targets for visualsign-solana Fix decode_instructions panics on malformed inputs Mar 15, 2026
@shahan-khatchadourian-anchorage shahan-khatchadourian-anchorage force-pushed the shahankhatch/add-fuzz-targets branch 3 times, most recently from 47efa1b to 0b545b9 Compare March 25, 2026 15:02
@shahan-khatchadourian-anchorage
Copy link
Copy Markdown
Contributor

Closing since this was just a small experiment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants