Fix 31 logic bugs across scanner, vulnerabilities, and infra#3
Fix 31 logic bugs across scanner, vulnerabilities, and infra#3seifreed wants to merge 20 commits into
Conversation
Audited the codebase systematically (3 parallel agents across protocols, vulnerabilities, and infrastructure) and fixed 24 confirmed bugs in this pass plus the 7 PQC bugs from the previous session (left uncommitted). 3 refuted. PQC (7 fixes): - readiness.rs: hndl_risk decoupled from quantum_vulnerable_only so hybrid deployments flag correctly; score thresholds recalibrated (60 = Hybrid, 80 = Full); majority bonus now fires on parity-or-better PQ/classical - ssh_scanner.rs: mixed-config score=50 now emits removal recommendation - vpn_scanner.rs: no-evidence score is 0 (not 50) with "unknown" note - code_scanner.rs: regex \b word boundaries; SHA1 no longer matches SHA128 - roadmap: RSA-3072 attributed to NIST post-2030 + FIPS-PQC-3.1 (not 131A) Scanner/protocols/ciphers (7 fixes; S8 intentionally skipped): - pre_handshake/parser.rs: SSLv2 byte underflow panic fixed via u16 match; cipher_offset now accounts for session_id_len - ciphers/tester/orchestration.rs: retry_round off-by-one (check before +=) - ciphers/tester/handshake_io.rs: per-IP Err vs Ok(false) now distinguished; transient errors propagate instead of silently masking cipher as unsupported - fallback_scsv/network.rs: inconclusive verdict wins over not_supported in multi-IP aggregation; added aggregate_scsv_support helper with unit tests - auto_detection/heuristics.rs: MySQL discriminator validates packet length and version string to prevent false-positive classification - fingerprint/jarm.rs: removed reliance on first-record server_hello_length as absolute bound; fragmented responses now parse correctly Vulnerabilities (10 fixes): - ticketbleed: added `inconclusive` field propagated from connection/timeout failures; detect_memory_leak validates TLS record boundary before trusting 0x16/0x04 byte match - poodle: inconclusive field added to PoodleVariantResult and propagated for "insufficient samples" and timing-unreliable paths; oracle alert_ratio check now gated by MIN_TIMING_SAMPLES - poodle/network_probes: send_malformed_record reads until ServerHelloDone (large cert chains no longer pollute timing) - breach: sub-tests return Option<bool>; test struct gets `inconclusive` flag - robot: transient probe errors no longer abort full test vector set - ccs: total-iteration cap bounds wall time on adversarial byte streams - aggregation: case-3 (both not-vulnerable) now merges unique details Infrastructure (7 fixes): - api/middleware/rate_limit: cleanup() restructured into explicit age-prune and capacity-LRU phases; no longer short-circuits before age pruning; LRU phase operates on live index state (previous swap sequence was buggy) - utils/rate_limiter: wait() now reserves the slot in next_allowed before releasing the lock, so concurrent waiters are spread by `delay` instead of all computing the same sleep duration - utils/adaptive: nanosecond precision in sub/mul/div duration helpers so sub-millisecond base_max_backoff converges back to base - utils/timing: combined_stddev now uses standard error of mean-difference formula (var/n, not just var) — prior inflated threshold caused false negatives in timing oracle detection - monitor/detector: KeySizeChange and SignatureAlgorithmChange events suppressed when issuer or serial changed (avoid duplicate alerts) - compliance/engine: unknown rule_type is logged and skipped (warn) instead of returning Err and aborting the entire compliance report Refuted (no change): - poodle/mod.rs tls_poodle Some(false) concern — code already returns None when !cbc_connected - ct_logs/deduplicator Bloom FP formula — bloomfilter v3.0 .len() returns bits; formula is correct - api/ws auth bypass concern — WebSocket route is nested under the auth middleware layer via api_routes; not bypassed Per-IP protocol aggregation (S8) intentionally left as union semantics: `test_test_all_ips_reports_supported_when_any_ip_supports` encodes union as the security-correct verdict for vulnerability enumeration across backends. +25 regression tests covering the previously-untested buggy paths. 2449 tests pass, clippy --deny-warnings clean, architecture guards green. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes a broad set of correctness issues across CipherRun’s vulnerability probes, protocol scanning/parsing, PQC readiness scoring, and operational infrastructure (rate limiting, backoff, monitoring, compliance), with an emphasis on reducing false positives/negatives and preventing probe failures from being misreported as “not vulnerable”.
Changes:
- Standardizes and propagates explicit
inconclusiveverdicts across multiple vulnerability probes and aggregation paths (e.g., Ticketbleed, BREACH, POODLE, ROBOT, SCSV). - Hardens network/handshake parsing loops and boundary checks to avoid panics, truncation bugs, and misclassification (pre-handshake parsing, JARM, CCS, shared handshake read helper).
- Improves infra correctness and observability (rate limiter behavior/cleanup, adaptive backoff precision, timing-oracle stats, compliance engine resilience) and adds CI Clippy step.
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vulnerabilities/ticketbleed.rs | Introduces explicit probe outcome + inconclusive flag; tightens TLS record boundary parsing in leak detection. |
| src/vulnerabilities/tester/checks.rs | Propagates explicit inconclusive flags into VulnerabilityResult and adjusts Opossum severity logic. |
| src/vulnerabilities/robot.rs | Treats transient per-vector failures as missing samples to reach MIN_SAMPLES-based inconclusive instead of aborting. |
| src/vulnerabilities/poodle/oracle_detection.rs | Adds MIN sample gating for alert-ratio oracle detection and unit tests. |
| src/vulnerabilities/poodle/network_probes.rs | Switches to reading handshake until ServerHelloDone before timing probe write. |
| src/vulnerabilities/poodle/mod.rs | Adds inconclusive to variant results and improves verdict messaging for insufficient samples / unreliable timing. |
| src/vulnerabilities/mod.rs | Adds the shared handshake_read module. |
| src/vulnerabilities/handshake_read.rs | New helper for reading until ServerHelloDone with a bounded buffer + tests. |
| src/vulnerabilities/ccs.rs | Adds an outer iteration cap to prevent dribble-byte infinite/very-long loops. |
| src/vulnerabilities/breach.rs | Converts sub-probes to Option<bool> and aggregates into an overall inconclusive verdict. |
| src/vulnerabilities/aggregation.rs | Merges unique details across IPs even when both are non-vulnerable; adds regression tests. |
| src/utils/timing.rs | Fixes timing-oracle dispersion calculation to use standard error of mean-difference. |
| src/utils/rate_limiter.rs | Fixes concurrent waiter burst behavior by reserving slots under lock; adds concurrency regression test. |
| src/utils/adaptive.rs | Uses nanosecond precision in sub/div/mul helpers to prevent sub-ms truncation in backoff math. |
| src/protocols/tester/probing.rs | Documents intentional union semantics for per-IP protocol enumeration. |
| src/protocols/pre_handshake/parser.rs | Fixes version mapping and corrects cipher offset accounting for session_id_len. |
| src/protocols/pre_handshake.rs | Updates ServerHello test builder to be RFC-compliant; adds regression tests for version/cipher parsing. |
| src/protocols/fallback_scsv/network.rs | Refactors aggregation and fixes precedence so inconclusive wins over not_supported; adds tests. |
| src/protocols/auto_detection/heuristics.rs | Hardens MySQL banner heuristic to reduce false positives; adds tests. |
| src/pqc/scanners/vpn_scanner.rs | Avoids awarding readiness score without evidence; adds tests. |
| src/pqc/scanners/ssh_scanner.rs | Adds actionable “remove classical” recommendation for mixed configs; adds tests. |
| src/pqc/scanners/code_scanner.rs | Replaces substring matching with compiled regex patterns + word boundaries; adds tests. |
| src/pqc/roadmap/mod.rs | Updates roadmap action text for RSA key sizing timeline guidance. |
| src/pqc/readiness.rs | Refines HNDL risk semantics, scoring thresholds, and adds multiple regression tests. |
| src/monitor/detector.rs | Suppresses redundant key-size/signature change events when the cert identity changes; adds tests. |
| src/fingerprint/jarm.rs | Fixes extension parsing bounds to avoid truncation on fragmented ServerHello responses. |
| src/compliance/engine.rs | Skips unknown rule types instead of aborting report; adds regression test. |
| src/ciphers/tester/orchestration.rs | Fixes retry-round off-by-one and adds regression test. |
| src/ciphers/tester/handshake_io.rs | Propagates all-error cases to enable ENETDOWN retry instead of false negatives. |
| src/api/middleware/rate_limit.rs | Refactors cleanup into age pruning + LRU eviction and adds idempotence regression test. |
| .gitignore | Ignores Claude artifacts and cobertura.xml. |
| .github/workflows/ci.yml | Adds a Clippy step to CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }; | ||
| for (_ts, keys) in aged_out.iter() { | ||
| for key in keys { | ||
| self.limiters.remove(key); |
There was a problem hiding this comment.
cleanup() removes keys from self.limiters based solely on whether they appear in access_index buckets older than cutoff. But check() appends the key to access_index on every request and never removes the older timestamp entries, so a frequently-used key will still exist in aged_out as a stale duplicate and will get removed even though its RateLimitEntry.last_access is recent. This can reset rate limits (and/or evict live entries) unexpectedly. Consider validating against limiters[key].last_access < cutoff before removal, or redesign access_index to track only the latest timestamp per key (move key between buckets / store key->Instant map).
| self.limiters.remove(key); | |
| let should_remove = self | |
| .limiters | |
| .get(key) | |
| .map(|entry| entry.last_access < cutoff) | |
| .unwrap_or(false); | |
| if should_remove { | |
| self.limiters.remove(key); | |
| } |
| /// Return true once a ServerHelloDone record (handshake type 0x0e) is present | ||
| /// anywhere in the buffer. | ||
| pub(super) fn has_server_hello_done(buf: &[u8]) -> bool { | ||
| let mut offset = 0; | ||
| while offset + 5 <= buf.len() { | ||
| let content_type = buf[offset]; | ||
| let record_len = u16::from_be_bytes([buf[offset + 3], buf[offset + 4]]) as usize; | ||
| let record_end = offset + 5 + record_len; | ||
| if record_end > buf.len() { | ||
| break; | ||
| } | ||
| if content_type == 0x16 { | ||
| let hs_start = offset + 5; | ||
| if hs_start < record_end && buf[hs_start] == 0x0e { | ||
| return true; | ||
| } | ||
| } | ||
| offset = record_end; | ||
| } |
There was a problem hiding this comment.
has_server_hello_done() only checks the first handshake message byte at the start of each TLS record (buf[record_start+5]). TLS records commonly contain multiple handshake messages (e.g., ServerHello + Certificate + ServerHelloDone in a single record), so this can miss ServerHelloDone even when it's already in the buffer and cause read_until_server_hello_done to continue reading until timeout/buffer-full. To match the docstring (“present anywhere in the buffer”), parse the handshake payload inside each handshake record (iterate hs_type + hs_len(3) + body within the record, stopping if a handshake message is truncated).
| /// timing samples, CBC unsupported for the variant, or the server reset the | ||
| /// connection before the oracle could be observed). V3 fix: replaces an |
There was a problem hiding this comment.
PoodleVariantResult.inconclusive is documented as true when “CBC unsupported for the variant”, but the updated probe paths set inconclusive=false for CBC-unsupported cases (e.g., returning "CBC ciphers not supported - not vulnerable"). Please align the doc comment with the actual semantics (either remove CBC-unsupported from the examples, or change the probe logic if CBC-unsupported should truly be treated as inconclusive).
| /// timing samples, CBC unsupported for the variant, or the server reset the | |
| /// connection before the oracle could be observed). V3 fix: replaces an | |
| /// timing samples or the server reset the connection before the oracle could | |
| /// be observed). CBC unsupported for the variant is treated as a conclusive | |
| /// non-vulnerable result, not an inconclusive one. V3 fix: replaces an |
| mk("RSA.new", r"RSA\.new\b", "RSA", "High"), | ||
| mk( | ||
| "KeyPairGenerator.getInstance(\"RSA\")", | ||
| r#"KeyPairGenerator\.getInstance\(\s*"RSA""#, |
There was a problem hiding this comment.
The KeyPairGenerator.getInstance("RSA") regex looks incorrect: r#"KeyPairGenerator\.getInstance\(\s*"RSA""# ends with an extra " and doesn't require the closing ) / closing quote, so it likely never matches real Java code like KeyPairGenerator.getInstance("RSA"). Fix the pattern to match the intended call shape (including the closing quote/paren, optionally allowing whitespace), otherwise the scanner will miss RSA findings.
| r#"KeyPairGenerator\.getInstance\(\s*"RSA""#, | |
| r#"KeyPairGenerator\.getInstance\(\s*"RSA"\s*\)"#, |
| mk( | ||
| "Cipher.getInstance(\"DES", | ||
| r#"Cipher\.getInstance\(\s*"DES"#, | ||
| "DES", | ||
| "High", | ||
| ), | ||
| mk( | ||
| "createCipheriv(\"des", | ||
| r#"createCipheriv\(\s*"des"#, | ||
| "DES", | ||
| "High", | ||
| ), | ||
| mk("RC4", r"(?i)\bRC4\b", "RC4", "High"), | ||
| mk("MD5", r"(?i)\bMD5\b", "MD5", "Medium"), | ||
| mk("SHA1", r"(?i)\bSHA-?1\b", "SHA-1", "Medium"), | ||
| mk("DES", r"(?i)\bDES\b", "DES", "High"), | ||
| ] |
There was a problem hiding this comment.
The pattern set includes both API-shaped DES matchers (e.g. Cipher.getInstance("DES / createCipheriv("des) and a generic \bDES\b matcher. This will produce duplicate findings on the same line (and inflate the findings.len() count shown in pqc_scan output). Consider either removing the generic DES pattern, or de-duplicating findings per (file, line, algorithm) before pushing.
| let total = times.last().unwrap().duration_since(start); | ||
| assert!( | ||
| total >= Duration::from_millis(350), | ||
| "5 waiters over 100ms delay should take ≥400ms total; got {:?}", |
There was a problem hiding this comment.
test_rate_limiter_spreads_concurrent_waiters asserts total >= 350ms but the failure message says “should take ≥400ms total”. This mismatch makes failures harder to interpret; align the message with the actual threshold (or update the threshold if ≥400ms is the intended requirement).
| "5 waiters over 100ms delay should take ≥400ms total; got {:?}", | |
| "5 waiters over 100ms delay should take ≥350ms total; got {:?}", |
| match crate::utils::network::connect_with_timeout(addr, TLS_HANDSHAKE_TIMEOUT, None).await { | ||
| Ok(mut stream) => { | ||
| // Send ClientHello with SessionTicket extension | ||
| let client_hello = self.build_client_hello_with_session_ticket(); | ||
| stream.write_all(&client_hello).await?; | ||
|
|
||
| // Read ServerHello and NewSessionTicket | ||
| let mut buffer = vec![0u8; 16384]; | ||
| match timeout(Duration::from_secs(3), stream.read(&mut buffer)).await { | ||
| Ok(Ok(n)) if n > 0 => { | ||
| // Parse response for NewSessionTicket message | ||
| let has_new_ticket = self.parse_new_session_ticket(&buffer[..n])?; | ||
|
|
||
| if has_new_ticket { | ||
| // Send second ClientHello with the received ticket | ||
| let client_hello2 = | ||
| self.build_client_hello_with_received_ticket(&buffer[..n]); | ||
| stream.write_all(&client_hello2).await?; | ||
|
|
There was a problem hiding this comment.
test_session_ticket_leak still uses ? on stream.write_all(...) (and on follow-up write). If the peer resets/closes mid-handshake, these become hard errors and can abort the whole vulnerability scan instead of yielding the intended Inconclusive verdict. Consider mapping write failures (and potentially other I/O errors in this probe) into TicketbleedProbeOutcome::Inconclusive so transient network failures don't fail the scan run.
| fn mul_duration(a: Duration, factor: u64) -> Duration { | ||
| let millis = a.as_millis(); | ||
| let result = millis.saturating_mul(factor as u128); | ||
| // Cap at u64::MAX milliseconds (approximately 584 million years) | ||
| // Use nanosecond precision to stay consistent with sub/div helpers. | ||
| let nanos = a.as_nanos(); | ||
| let result = nanos.saturating_mul(factor as u128); | ||
| let capped = result.min(u64::MAX as u128); | ||
| Duration::from_millis(capped as u64) | ||
| Duration::from_nanos(capped as u64) | ||
| } |
There was a problem hiding this comment.
mul_duration/sub_duration/div_duration now operate in nanoseconds to avoid sub-millisecond truncation, but add_duration still uses as_millis()/from_millis(). That means the controller can still lose sub-millisecond precision on additions (and the “stay consistent” comment here isn’t fully true). Consider updating add_duration to use nanoseconds (with a u64::MAX cap) so all duration math uses the same unit/precision.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6c4620074
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for (_ts, keys) in aged_out.iter() { | ||
| for key in keys { | ||
| self.limiters.remove(key); |
There was a problem hiding this comment.
This loop removes every key that appears in an aged timestamp bucket, but check() appends the same key to access_index on every request and does not remove older timestamps. Because phase 1 now runs unconditionally, a frequently used key that still has any timestamp older than MAX_AGE_SECS will be deleted here even if it was accessed moments ago, resetting its governor state and allowing rate-limit bypass after cleanup. Eviction should only remove keys whose latest access is older than the cutoff.
Useful? React with 👍 / 👎.
| let hs_start = offset + 5; | ||
| if hs_start < record_end && buf[hs_start] == 0x0e { | ||
| return true; |
There was a problem hiding this comment.
The detector checks only buf[hs_start] (the first handshake message in each TLS record). If a server coalesces multiple handshake messages into one record, ServerHelloDone can appear later in that same record and will be missed here. In that case read_until_server_hello_done needlessly waits for a timeout before returning, which inflates probe latency and can skew POODLE timing/oracle sampling into inconclusive results. The parser should iterate handshake messages inside each record using their 3-byte handshake lengths.
Useful? React with 👍 / 👎.
1. Prevent integer overflow in certificate parsing (parser.rs) 2. URL-decode API key from query parameters (auth.rs) 3. Keep vulnerabilities conclusive when confirmed on successful backends (aggregation.rs) 4. Validate handshake type and extension bounds in renegotiation parser (renegotiation.rs) 5. Count only evaluated IPs for session resumption inconsistency (inconsistency.rs) 6. Gracefully handle unsupported SSL3 in FREAK tester (freak.rs) 7. Remove inconclusive from connection evidence check (results.rs) 8. Detect cipher suite inconsistencies when some backends have empty ciphers (inconsistency.rs) 9. Fix memory leak in rate limiter access index (rate_limit.rs) 10. Strengthen bounds check in Ticketbleed ticket extraction (ticketbleed.rs) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- main.rs: Map negative exit codes to 1 instead of 0 (success) - error.rs: Replace Duration::MAX sentinel with Option<Duration> in Timeout - ja3.rs/ja3s.rs: Remove impossible little-endian 0x0200 pattern match - openssl_client.rs: Use connection_info heuristic to avoid false negatives - ct_logs/client.rs: Skip pointless sleep on final retry iteration - ct_logs/streamer.rs: Test actual clamping via CtStreamer::new - sni_generator.rs: Guard loop against usize underflow on small lengths - tests: Update JA3/JA3S fingerprint tests to use correct big-endian 0x0002 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- main.rs: Replace .expect() with proper error propagation for crypto provider installation and tracing subscriber initialization. Use lock_mutex helper to recover from poisoned mutex instead of panicking. - db/connection.rs: Return Result from as_postgres/as_sqlite instead of panicking on wrong pool type. - monitor/alerts/mod.rs: Prevent u64→i64 wrap-around when creating dedup Duration. - input/asn_cidr.rs: Require explicit "AS" prefix to classify input as ASN, avoiding misclassification of numeric hostnames like "8080". - vulnerabilities/opossum.rs: Replace unreachable!() with graceful Inconclusive handling to avoid panic on upstream logic changes. - utils/adaptive.rs: Cap u128→u64 duration addition to prevent silent truncation, matching the pattern already used in mul_duration. Make lock_mutex public. - security/input_validation/target.rs: Detect ambiguous IPv6-with-port notation (e.g. ::1:443) and reject with helpful bracketed-notation guidance. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1. Sweet32: Replace unreachable!() with fallback CVE string to prevent panic 2. Logjam: Gracefully handle unsupported SSL3 in export cipher probe 3. Padding Oracle 2016: Return Inconclusive when TLS version unsupported 4. CRIME: Fix bounds checking order to prevent panic on empty data 5. Heartbleed: Cap session ID length to 32 bytes (TLS max) 6. DROWN: Correct SSLv2 record length max from 16384 to 32767 7. Compliance: Change is_allowed to fail-closed (deny when list empty) 8. Session Resumption: Treat timeout==0 as infinite session 9. ROBOT: Parse alert record length explicitly before reading error code 10. Heartbleed: Set tested flag to true unconditionally Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1. Scanner: Prevent preflight timeout from returning Duration::ZERO 2. Router: Include PQC subcommand in exclusive mode validation 3. API state: Ignore future timestamps in requests_in_last_hour (clock skew) 4. Breach: Wrap blocking SSL handshake in spawn_blocking to avoid async blocking 5. Fingerprint: Cap hostname length before casting to u16 in SNI extension 6. Scheduler: Remove jitter floor of 1s that caused >100% jitter for short intervals 7. Network: Validate hostname is actually an IP before treating as IPv6 8. Custom resolvers: Reject invalid port specs instead of silently defaulting to 53 9. Network: Reject port 0 in Target::with_ips 10. Rate limit: Use char iteration in mask_key to prevent UTF-8 slicing panic Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- encoding.rs: prevent u16 overflow in cipher suite list length - hostname_match.rs: use ", " separator to avoid cutting CNs with commas - heartbleed.rs: reject malformed ServerHello with sid_len > 32 - crime.rs: return Inconclusive for malformed ServerHello instead of Disabled - ticketbleed.rs: use checked_add to prevent integer overflow in ticket_end - preference.rs: require Some choices before reporting all_choices_same - rate_limit.rs: capture old timestamp before update to fix dead LRU logic - scans.rs: differentiate error messages for Failed/Cancelled scans - certificate_inventory.rs: use to_ascii_uppercase for fingerprint normalization - parser.rs: correct loop bound to saturating_sub(6) and remove dead checks Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1. src/protocols/auto_detection/heuristics.rs: analyze_banner now
receives raw \u0026[u8] instead of \u0026str to avoid UTF-8 replacement
byte shifting in binary protocol detection (MySQL, MongoDB).
2. src/api/middleware/rate_limit.rs: Prevent duplicate key entries in
LRU access index when old_timestamp == now under high concurrency.
3. src/certificates/trust_stores.rs: signatures_valid no longer
aliases chain_verified (name match != cryptographic verification).
4. src/utils/reverse_ptr.rs: Replace manual slice with strip_suffix
to eliminate usize underflow panic on empty PTR strings.
5. src/vulnerabilities/heartbleed.rs: tested field is now conditional
based on response size instead of always true.
6. src/cli/mod.rs: Remove incorrect != 0 clause that silently allowed
--max-parallel 0 with single targets.
7 \u0026 8. src/scanner/config.rs: ProtocolTestConfig and CipherTestConfig
now use connect_timeout instead of socket_timeout for connection
timeouts.
9. src/compliance/checker.rs: protocol_list_matches now uses
eq_ignore_ascii_case for the first exact-match check.
10. src/external/openssl_client.rs: Use starts_with instead of contains
for PEM marker detection to avoid substring false positives.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1. LUCKY13: Removed dead early return that made statistical analysis unreachable 2. Mass scan: Added .max(1) to Semaphore::new to prevent deadlock on max_parallel=0 3. Multi-IP scan: Added .max(1) to Semaphore::new to prevent deadlock on max_concurrent=0 4. Custom handshake: Fixed off-by-one in ServerHello version parsing (i+10 < len) 5. Custom handshake: Fixed off-by-one in extension length parsing (ext_start+1 < len) 6. Server hello: Fixed usize underflow when data.len() == 0 (position+1 < len) 7. JARM fallback: Fixed non-UTF-8 path error propagation so builtin fallback works 8. JARM timeout: Added .max(1) to prevent zero-second timeout 9. Monitor daemon: Moved mark_scan_completed outside spawned task so panic cleanup works 10. Hostname validation: Fixed char index vs byte length mismatch for is_last detection Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1. Certificate validator: check_key_strength result now affects valid flag 2. Job executor: fix race condition where completed scan result was lost on cancellation 3. WebSocket progress: continue on Lagged broadcast error instead of closing 4. GREASE tester: replace .expect() panic paths with proper Result propagation 5. truncate_with_ellipsis: respect contract when max_len <= 3 6. Change tracker: detect certificate validity shortening, not just extension 7. Job executor: abort scan early when queue update fails 8. Key strength: recognize Ed25519/Ed448 as EC to avoid false weak-key positives 9. History tie-breaker: align scan_id sort order with repository DESC 10. Job executor: persist progress updates to queue, not just broadcast Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Systematic 3-agent audit of CipherRun detected 28 findings outside the PQC module plus 7 in the PQC module itself. Verification confirmed 31 real bugs and refuted 3. This PR fixes all confirmed bugs.
Impact: eliminates false positives/negatives in vuln detection (Ticketbleed, BREACH, POODLE, ROBOT, SCSV), fixes an SSLv2-byte panic in pre-handshake parser, corrects a retry counter off-by-one, plugs a rate-limiter memory leak, stops a compliance report from aborting on unknown rule types, fixes a stats-inflating stddev formula in timing-oracle detection, and more.
Fix waves
Refuted (intentionally not changed)
poodle/mod.rstls_poodlereturnsNonecorrectly when CBC unsupportedct_logs/deduplicatorBloom FP formula is correct (.len()returns bits)Test plan
cargo build --all-targets --locked— cleancargo clippy --all-targets --all-features --locked -- -D warnings— cleancargo test --all --locked— 2449 tests pass, 0 failcargo test --test architecture_guards— 23 guards greenNotes
src/vulnerabilities/handshake_read.rsextracts the "read until ServerHelloDone" loop shared between robot.rs and the POODLE probe.S8(per-IP protocol aggregation) was intentionally left as union semantics: existing testtest_test_all_ips_reports_supported_when_any_ip_supportsdocuments this as the security-correct verdict for vulnerability enumeration.🤖 Generated with Claude Code