Skip to content

SimplrSSH - Bug Hunter - 2026-10-17 - #25

Merged
bluenevus merged 1 commit into
integrationfrom
bug-hunt/20261017-0000
Oct 17, 2026
Merged

bluenevus merged 1 commit into
integrationfrom
bug-hunt/20261017-0000

Conversation

@bluenevus

Copy link
Copy Markdown
Contributor

Findings

  1. Host-key records read into a fixed 160-byte buffer (main/known_hosts.cpp, check_host_key and load_known_hosts; root cause: char value[160] with nvs_get_str).
    • A saved record is host:port, key type, SHA256 fingerprint, about 65 bytes plus the host name. A host name of roughly 85 characters or more makes the read fail with ESP_ERR_NVS_INVALID_LENGTH.
    • check_host_key then returns Unknown instead of Match or Mismatch. So a changed (possibly MITM) key for that server shows as a "New server" trust prompt, not a key-changed refusal, and hosts omits the entry.
    • Severity: Medium (security downgrade for long host names; host input length is unbounded). It affects users who SSH to long FQDNs.

Reviewed and found no defects: vterm.cpp, ui_page.cpp, command_redact.cpp, connection_profiles.cpp and the command dispatch in ssh_terminal.cpp.

Plan

  • Add read_record() in known_hosts.cpp: size query, then heap read, bounded to 1 KiB. Use it in both places.
  • Add a guard test that fails if a fixed char value[N] buffer returns in known_hosts.cpp.

Final gap analysis

  • read_record() replaces both fixed-buffer reads: done.
  • Guard test: done. known_hosts.cpp depends on libssh2 and SSHTerminal, so it is not in the host test build. A behavioural unit test would need a refactor, so I added a source guard instead.

Deferred

  • Host and input length limits are not enforced anywhere (current_input is unbounded). This needs a product decision on the limits.

Verification

  • scripts/check.sh with SKIP_SAST=1:
    • Guards: 95 tests pass.
    • Host unit tests (ctest -L unit): pass, with the sanitizer build.
    • SAST was not run locally (no docker/ECR). CI's security scan still runs it.
  • Not built with ESP-IDF, and not tested on hardware. The firmware build and a real T-Deck or Tab5 need confirming.
  • RAM delta: none expected. It is a transient heap string, not static.

Generated by Claude Code

…or a new server

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AZQ79Khe8LKPgeyn474r1i
@bluenevus
bluenevus merged commit 1c02453 into integration Oct 17, 2026
32 checks passed
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