Hi Damien,

Damien Miller wrote on Thu, Aug 13, 2026 at 11:42:33AM +1000:

> This allows UTF-8 characters in log messages, either destined to the
> TTY or to syslog IFF ssh/sshd is started in a UTF-8 locale. It will
> fall back to strnvis() sanitisation of output strings in other cases.

Actually, the fallback to vis(3) occurs:

 1. In an UTF-8 locale, for bytes that do not form valid UTF-8 syntax.
 2. In an UTF-8 locale, for valid UTF-8 characters that are not
    printable, i.e. that have a negative wcwidth(3), except for
    L'\n', L'\r', and L'\t', which are considered printable
    even though they have wcwidth(3) == -1.
 3. In an US-ASCII locale, for bytes that are not valid ASCII,
    i.e. those outside the range from 0x00 to 0x7f inclusive.
 4. In an US-ASCII locale, for bytes that are not printable,
    i.e. everything except '\n', '\r', '\t', and 0x20 to 0x7e inclusive.
 5. In a dangerous locale (i.e. anything that is neither US-ASCII
    nor UTF-8), vis(8)-escaping is never attempted because splitting
    and assembling strings is impossible in general.  Instead, if any
    syntax or printability problem is encountered in a dangerous
    locale, the logged string is truncated at that point and
    the function snmprintf() returns -1 to indicate an
    unrecoverable output error.
    Note that it would NOT be possible to print something like
    "OUTPUT TRUNCATED" to the log file at that point.  Instead, the
    log file must be considered unrecoverably corrupted.  It must
    be closed, deleted, and opened anew from scratch.  If that seems
    unacceptable to you (it certainly would to me), just don't be
    so stupid as to specify that your log files shall be written
    in a locale other than US-ASCII or UTF-8.

> ok?
> 
[...]
> index 74ecd5b..2fad5e2 100644
> --- a/log.c
> +++ b/log.c
> @@ -45,10 +45,10 @@
>  #include <string.h>
>  #include <syslog.h>
>  #include <unistd.h>
> -#include <vis.h>
>  
>  #include "log.h"
>  #include "match.h"
> +#include "utf8.h"
>  
>  static LogLevel log_level = SYSLOG_LEVEL_INFO;
>  static int log_on_stderr = 1;
> @@ -361,12 +361,12 @@ do_log(LogLevel level, int force, const char *suffix, const char *fmt,
>  		snprintf(fmtbuf, sizeof(fmtbuf), "%s: %s", msgbuf, suffix);
>  		strlcpy(msgbuf, fmtbuf, sizeof(msgbuf));
>  	}
> -	strnvis(fmtbuf, msgbuf, sizeof(fmtbuf), VIS_SAFE|VIS_OCTAL);
> +	snmprintf(fmtbuf, sizeof(fmtbuf), NULL, "%s", msgbuf);
>  	if (log_handler != NULL) {
>  		/* Avoid recursion */
>  		tmp_handler = log_handler;
>  		log_handler = NULL;
> -		/* Note: this sends the raw (i.e. no strnvis) log message */
> +		/* Note: this sends a raw (i.e. no sanitisation) log message */
>  		tmp_handler(level, force, msgbuf, log_handler_ctx);
>  		log_handler = tmp_handler;
>  	} else if (log_on_stderr) {

Judging purely from code inspection, this seems reasoable to me,
assuming it is intentional that you silently truncate the logged
string in case vis(3)ing makes it so much longer that it no longer
fits inside the fmtbuf.  If push comes to shove, that might enable
an attacker to hide information they don't want the system administrator
to know from the logs by injecting sufficient amounts of non-printable
characters in front of the information to be hidden.  Actually, this
might already be possible with the current strnvis(3) sanitation.

While i did review the diff, I'm not planning to test.

Given how bad it would be if some stupid system administrator
would start sshd(8) in a locale other than US-ASCII or UTF-8 -
essentially, that would invite attackers to disable logging by
injecting invalid bytes into the logging stream, and hence hide
other attacks - it might make sense for sshd(8) to refuse startup
if it detects a dangerous_locale().

I don't think that's a prerequisite for this diff though; it can
be considered separately.  You already document somewhere that
locales other than US-ASCII and UTF-8 are unsafe and should never
be used with SSH, neither server- nor client-side, right?

Yours,
  Ingo