Bound CSI continuation parse and clear esc state on completion

[yosuke-wolfssl]

Overview

Fixes a one-byte heap out-of-bounds read in wolfSSH_DoControlSeq (src/wolfterm.c) when a Linux-console CSI escape sequence is split across SSH channel-data packets such that a continuation buffer contains only argument bytes with no command terminator. While in this code, also hardens the escBuf reassembly bounds and fixes a related escape-state leak in the resume path. Windows-only code (USE_WINDOWS_API).

Addressed by f_5903.

The bug

wolfSSH_DoControlSeq is re-entered with ssh->escState == WS_ESC_CSI and ssh->escBufSz == 0 when a prior channel-data buffer ends exactly after ESC [. On the next call, the outer-if is taken but the inner-if (escBufSz > 0) is false, falling into an unguarded inner-else:

else {
    numArgs = getArgs(buf, bufSz, &i, args);
    c = buf[i]; i++;   /* OOB: getArgs advanced i to bufSz */
}

When the continuation buffer holds only argument bytes (digits/semicolons) and no command char, getArgs advances i to bufSz, so c = buf[i] reads one byte past the allocation. The sibling outer-else branch already guarded this exact dereference with if (i >= bufSz); the inner-else did not.

• Impact: one-byte heap OOB read (CWE-125), ASan-confirmed.
• Reachability: USE_WINDOWS_API build, authenticated peer controlling SSH channel-data packet boundaries.

Changes

1. Bound the CSI continuation read. Add the same if (i >= bufSz) reassembly guard the sibling branch uses — stash the partial sequence in escBuf and return WS_WANT_READ instead of reading past the buffer.

2. Explicit escBuf capacity bound. Both escBuf saves now reject bufSz - *idx >= WOLFSSL_MAX_ESCBUF before the WMEMCPY, using this function's existing WLOG + WS_FATAL_ERROR idiom and matching the boundary semantics of the resume check. Defense-in-depth: in the default build getArgs caps the advance at WOLFSSH_MAX_CONSOLE_ARGS (16) < WOLFSSL_MAX_ESCBUF (19), so the copy was already safe; this prevents a custom WOLFSSH_MAX_CONSOLE_ARGS > WOLFSSL_MAX_ESCBUF build from overflowing escBuf.

3. Clear escape state on resume completion. When a split CSI/OSC sequence completes via the wolfSSH_ConvertConsole resume handlers, reset escState = WC_ESC_NONE (not just escBufSz), matching the inline completion path. Without this, escState stayed WS_ESC_CSI and the next call's leading plain bytes were fed back into CSI parsing instead of being printed.

Testing

Extended test_wolfSSH_ConvertConsole in tests/api.c:

• Split CSI across calls (ESC [ | 12 | m) — drives the previously-unguarded inner-else and verifies reassembly completes.
• Trailing plain byte after completion — verifies escState is cleared (returns WS_SUCCESS, not swallowed into a new CSI parse).
• Single-buffer args-to-end (ESC [ 1 2) — covers the initial-parse save branch with real content.

Verified on native Windows (Visual Studio Build Tools 2022, static Debug x64):

api-test EXITCODE=0

Negative controls confirm each test catches its regression:

Reverted fix	Result
OOB guard removed	api.c:3085 fails — csi_args returns WS_FATAL_ERROR instead of WS_WANT_READ
escState reset removed	api.c:3093 fails — trailing plain byte returns WS_WANT_READ instead of WS_SUCCESS

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1076

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

[aidangarske]

🐺 Skoll Code Review

Overall recommendation: REQUEST_CHANGES
Findings: 1 total — 1 posted, 0 skipped

Posted findings

• [High] CSI capacity errors are swallowed on resume — src/wolfterm.c:684-691

Review generated by Skoll.

[aidangarske]

🟠 [High] CSI capacity errors are swallowed on resume
🚫 BLOCK bug

The PR adds WS_FATAL_ERROR returns in wolfSSH_DoControlSeq when saving a CSI continuation would exceed escBuf, but the resume path in wolfSSH_ConvertConsole treats every non-WS_WANT_READ result as a completed sequence. For example, after ESC [ leaves ssh->escState == WS_ESC_CSI and escBufSz == 0, a following buffer of WOLFSSL_MAX_ESCBUF argument bytes hits the new bufSz - *idx >= WOLFSSL_MAX_ESCBUF guard and returns WS_FATAL_ERROR. This caller then clears the escape state, falls through with i still 0, writes those bytes as plain output, and returns WS_SUCCESS. That means the new defensive rejection path introduced by the PR is not actually enforced for the resume case it is meant to harden.

Suggestion:

Suggested change

	if (ret == WS_WANT_READ) {
	ret = wolfSSH_DoControlSeq(ssh, handle, buf, bufSz, &i);
	if (ret == WS_WANT_READ) {
	return ret;
	}
	if (ret != WS_SUCCESS) {
	return ret;
	}
	ssh->escState = WC_ESC_NONE;
	ssh->escBufSz = 0;
	break;