Skip to content

SimplrSSH - Bug Hunter - 2026-10-16 - #22

Merged
bluenevus merged 1 commit into
integrationfrom
bug-hunt/20261016-0000
Oct 16, 2026
Merged

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

Conversation

@bluenevus

Copy link
Copy Markdown
Contributor

Findings

1. Out-of-bounds page.fields[-1] in the sign-in page parser (main/portal.cpp, Parser::start_tag, select branch; reached from finish_option())

  • Root cause: <select> did not close an open <option>. If the new select added no field (disabled, or past MAX_FIELDS), select_field became -1 while in_option stayed true. The next </select>, <option> or the end of the page then ran finish_option(), which indexed page.fields[-1]. If the new select did add a field, the old option was filed under the wrong select.
  • Impact (medium): the HTML comes from whatever WiFi network the user joined, so it is untrusted. A hostile or broken captive portal could trigger the out-of-bounds access while the user runs portal: memory corruption, or a crash and reboot. It cannot be reached when the user does not run portal.
  • Fix: call finish_option() when a <select> starts.

Read with no defect found: vterm.cpp, ui_page.cpp, command_redact.cpp, tab5_keymap.cpp, and the URL, entity and tokenizer parts of portal.cpp.

Plan

  1. Close the open option when a new select starts (main/portal.cpp). Verify with a regression test under ASAN.

Gap analysis (final)

Item Status
Fix in portal.cpp Done
Regression test portal_unclosed_option_before_a_skipped_select Done. It fails under ASAN without the fix and passes with it.

Deferred

  • Not fixed, too minor and not exploitable: decode_entities accepts &# 65; and &#+65; because strtoul skips whitespace and signs. Browsers do not decode these.
  • Not fixed, edge case: colon sub-parameters in SGR, such as 38:2::r:g:b with an empty colour-space field, are treated as plain parameters and give a wrong colour.
  • Not audited this run (too large for one pass): ssh_terminal.cpp, chat_app.cpp, openai_client.cpp, known_hosts.cpp, settings_backup.cpp.

Verification

  • Baseline on integration, before the change: SKIP_SAST=1 scripts/check.sh passed (93 guard tests, host unit tests). I installed libmbedtls-dev first because the host build needs it.
  • With the new test and without the fix: the unit test fails under ASAN.
  • After the fix: SKIP_SAST=1 scripts/check.sh passed (guards OK, host unit tests 100% passed).
  • Not run: SAST (scripts/sast.sh), because docker/ECR is not available here. CI's security scan still runs it. The firmware build was not run either, since ESP-IDF is not installed. This change touches only host-testable parser code.

🤖 Generated with Claude Code

https://claude.ai/code/session_015W3Ue1tJJP4yetdkD4wmX1


Generated by Claude Code

An <option> left open before a <select> that adds no field (disabled, or past
MAX_FIELDS) was finished later with select_field == -1, indexing page.fields[-1].
Close the open option when a new select starts.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015W3Ue1tJJP4yetdkD4wmX1
@bluenevus
bluenevus merged commit 34958c2 into integration Oct 16, 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