Sanitize control bytes in formatted log messages

[yosuke-wolfssl]

Sanitize control bytes in formatted log messages

Summary

Network-supplied SSH protocol strings were logged verbatim through WLOG("%s", ...).
A peer could embed CR/LF or terminal control bytes to forge or corrupt log records (log injection). This scrubs control bytes once, centrally, in wolfSSH_Log, closing the whole class across every logging sink.

Problem

Parser routines (GetString, GetStringRef, GetStringAlloc) copy protocol fields verbatim.
Many wolfSSH_Log/WLOG sinks across src/internal.c, src/wolfsftp.c, src/wolfscp.c, and apps/wolfsshd/ then logged those fields with %s and no filtering. DefaultLoggingCb writes the formatted line with fprintf and no escaping, so an embedded \n splices forged log entries that are indistinguishable from genuine ones.
The service-name and SSH_MSG_DEBUG sinks are reachable pre-authentication.

Fix

Scrub the formatted message in wolfSSH_Log, after WVSNPRINTF and before the logging callback runs: replace any byte < 0x20 (except tab) and 0x7f (DEL) with ?.

for (i = 0; msgStr[i] != 0; i++) {
    c = (byte)msgStr[i];
    if ((c < 0x20 && c != '\t') || c == 0x7f)
        msgStr[i] = '?';
}

Why this layer:

• Every WLOG expands to wolfSSH_Log, and direct wolfSSH_Log callers
  (including wolfsshd) build their line here too, so one site covers all current
  and future sinks across all files and custom logging callbacks.
• It scrubs only the transient formatted copy; parsed protocol data is left
  byte-exact (so comparisons and consumed values such as env values, exec
  command, and filenames are unaffected). This is why the fix belongs here and
  not in the parsers.
• Tab is preserved; 8-bit/UTF-8 bytes are untouched, so legitimate non-ASCII
  filenames are not mangled.

Findings closed

Closes the log-injection family in one change: 4234, 4347, 4348,
4350-4353, 4662, 4721, 4797, 4944, 5113, 5689, 5854.

Tests

• New test_wolfSSH_Log_sanitize installs a capturing logging callback and
  asserts that CR/LF/ESC/DEL in a %s argument are scrubbed, tab is
  preserved, and clean strings pass through unchanged.
• Negative control: neutering the scrub condition makes the new test fail,
  confirming it detects regressions.

[Copilot]

Pull request overview

This PR mitigates log injection risks by escaping network-supplied SSH protocol strings at the logging sink (rather than mutating parsed protocol data), preventing CR/LF and control-byte injection into log records.

Changes:

• Adds wolfSSH_LogSafeStr() plus WLOG_SAFE* helper macros to escape untrusted strings before logging.
• Wraps multiple log sinks in src/internal.c that previously logged attacker-controlled %s data directly.
• Adds a unit test in tests/api.c covering escaping behavior, truncation, and edge cases.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File	Description
wolfssh/log.h	Declares the new escaping API and introduces WLOG_SAFE, WLOG_SAFE_U, and WLOG_SAFE2 macros.
src/log.c	Implements wolfSSH_LogSafeStr() used to sanitize untrusted strings for log output.
src/internal.c	Replaces vulnerable WLOG(... "%s", ...) call sites with WLOG_SAFE* wrappers for network-controlled fields.
tests/api.c	Adds a unit test validating escaping rules and safety/truncation guarantees.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1031

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅