On 10/07/2026 10:35, Pádraig Brady wrote:
Note I'm also looking at a follow up patch to
disallow or more probably replace ',' and \n in names
when not quoting, to give better protection against
malicious names.
Proposed patches for this attached.

cheers,
Padraig
From 26a36708f15d45926c31b77e5f74fa8ad26e6679 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Fri, 10 Jul 2026 15:42:34 +0100
Subject: [PATCH 1/2] ls: expand output separator protection

* doc/coreutils.texi (ls invocation): Expand --literal description.
* src/ls.c (quote_name_buf): Replace '\n' and ',' with '\n' if required.
* tests/ls/dired.sh: Add a test case.
* tests/ls/zero-option.sh: Likewise.
* tests/ls/m-option.sh: Likewise, and adjust existing test.
* NEWS: Mention the change in behavior.
---
 NEWS                    |  5 +++++
 doc/coreutils.texi      | 10 ++++++----
 src/ls.c                | 11 ++++++++++-
 tests/ls/dired.sh       | 13 +++++++++++++
 tests/ls/m-option.sh    | 10 +++++++++-
 tests/ls/zero-option.sh |  9 ++++-----
 6 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/NEWS b/NEWS
index d57a6441c..81f1ddf03 100644
--- a/NEWS
+++ b/NEWS
@@ -52,6 +52,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   'ls' -w,--width no longer includes '\n' in the width of a line.
   I.e., the width or $COLUMNS is interpreted to be an _inclusive_ maximum.
 
+  'ls' now protects file names from output separators when using literal
+  quoting: newlines are replaced with '?' unless --zero or --dired is active,
+  and with -m commas are likewise replaced.  Previously only newlines
+  were protected, and only when outputting to a terminal.
+
   'stat' now uses shell quoting when required, to more robustly escape
   file names.  Previously it only quoted file names with the %N format.
   The default quoting honors the QUOTING_STYLE env variable (like %N).
diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index 5cc831f84..858eec79f 100644
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -8464,10 +8464,12 @@ backslash sequences like those used in C.
 @optItemx{ls,--literal,}
 @itemx --quoting-style=literal
 @opindex --quoting-style
-Do not quote file names.  However, with @command{ls} nongraphic
-characters are still printed as question marks if the output is a
-terminal and you do not specify the @option{--show-control-chars}
-option.
+Do not quote file names.  However, newline characters are printed as
+question marks unless @option{--zero} or @option{--dired} is active.
+When @option{-m} comma separation is active, commas are likewise printed
+as question marks.  Other nongraphic characters are printed as question
+marks if the output is a terminal and you do not specify the
+@option{--show-control-chars} option.
 
 @optItem{ls,-q,}
 @optItemx{ls,--hide-control-chars,}
diff --git a/src/ls.c b/src/ls.c
index d54ef3d98..d9b7230ca 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -4469,6 +4469,10 @@ quote_name_buf (char **inbuf, size_t bufsize, char *name,
                                && (qs == shell_quoting_style
                                    || qs == shell_always_quoting_style
                                    || qs == literal_quoting_style);
+  bool needs_separator_protection
+    = (! dired && qs == literal_quoting_style
+       && ((eolbyte && strchr (name, '\n'))
+           || (format == with_commas && strchr (name, ','))));
 
   if (needs_general_quoting != 0)
     {
@@ -4481,7 +4485,7 @@ quote_name_buf (char **inbuf, size_t bufsize, char *name,
 
       quoted = (*name != *buf) || strlen (name) != len;
     }
-  else if (needs_further_quoting)
+  else if (needs_further_quoting || needs_separator_protection)
     {
       len = strlen (name);
       if (bufsize <= len)
@@ -4497,6 +4501,11 @@ quote_name_buf (char **inbuf, size_t bufsize, char *name,
       quoted = false;
     }
 
+  if (needs_separator_protection)
+    for (char *p = buf; p < buf + len; p++)
+      if ((eolbyte && *p == '\n') || (format == with_commas && *p == ','))
+        *p = '?';
+
   if (needs_further_quoting)
     {
       if (MB_CUR_MAX > 1)
diff --git a/tests/ls/dired.sh b/tests/ls/dired.sh
index 8b80b855b..1becde22d 100755
--- a/tests/ls/dired.sh
+++ b/tests/ls/dired.sh
@@ -76,5 +76,18 @@ while test "$#" -gt 0; do
   index=$(($index + 1))
 done
 
+# Ensure literal quoting preserves newlines for dired consumers.
+newline='n
+l'
+mkdir newline-dir || framework_failure_
+touch "newline-dir/$newline" || framework_failure_
+ls -l --dired --quoting-style=literal newline-dir > out || fail=1
+set -- $(sed -n 's|^//DIRED// ||p' out)
+test "$#" -eq 2 || framework_failure_
+dd bs=1 skip="$1" count="$(($2 - $1))" < out > actual 2>/dev/null \
+  || framework_failure_
+printf %s "$newline" > exp || framework_failure_
+compare exp actual || fail=1
+
 
 Exit $fail
diff --git a/tests/ls/m-option.sh b/tests/ls/m-option.sh
index afe3fd415..1769af482 100755
--- a/tests/ls/m-option.sh
+++ b/tests/ls/m-option.sh
@@ -55,7 +55,7 @@ compare exp out || fail=1
 # especially in the presence of NBSP etc.
 touch 'com,ma' || framework_failure_
 cat <<\EOF > exp || framework_failure_
-literal: com,ma
+literal: com?ma
 shell: 'com,ma'
 shell-always: 'com,ma'
 shell-escape: 'com,ma'
@@ -72,4 +72,12 @@ for qs in $(cut -d: -f1 exp); do
 done > out
 compare exp out || fail=1
 
+# Literal newlines are also protected when not using --zero.
+newline='n
+l'
+touch "$newline" || framework_failure_
+printf '%s\n' 'n?l' > exp || framework_failure_
+ls -1 --show-control-chars --quoting-style=literal "$newline" > out || fail=1
+compare exp out || fail=1
+
 Exit $fail
diff --git a/tests/ls/zero-option.sh b/tests/ls/zero-option.sh
index 8c06b5a04..aa053a0af 100755
--- a/tests/ls/zero-option.sh
+++ b/tests/ls/zero-option.sh
@@ -29,12 +29,11 @@ disallowed_options='-l --dired'  # dired only enabled with -l
 returns_ 2 ls $disallowed_options --zero dir || fail=1
 
 disabled_options='--color=always -x -m -C -Q -q'
+newline='n
+l'
+touch dir/'com,ma' "dir/$newline" || framework_failure_
 LC_ALL=C ls $disabled_options --zero dir >out || fail=1
-tr '\n' '\0' <<EOF >exp
-a
-b
-cc
-EOF
+printf '%s\0' a b cc 'com,ma' "$newline" >exp || framework_failure_
 
 compare exp out || fail=1
 
-- 
2.55.0

From 01be7e7032ab60dc115cad91b9ebc891b7781cd0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]>
Date: Fri, 10 Jul 2026 17:14:27 +0100
Subject: [PATCH 2/2] ls: support ISO-2022-JP in separator replacement
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* src/ls.c (quote_name_buf): Even though both newline and comma
are generally not part of modern multi-byte sequences, and newline
is not part of any multi-byte sequence, comma may be part of e.g.
が (U+304C) in its ISO-2022-JP representation.
Move the replacement into the multi-byte iteration logic,
and refactor that a little to avoid repetition.
* tests/ls/quoting-utf8.sh: Add a test case.
---
 src/ls.c                 | 103 +++++++++++++++++++++++----------------
 tests/ls/quoting-utf8.sh |   9 ++++
 2 files changed, 70 insertions(+), 42 deletions(-)

diff --git a/src/ls.c b/src/ls.c
index d9b7230ca..fd77a243a 100644
--- a/src/ls.c
+++ b/src/ls.c
@@ -4501,12 +4501,7 @@ quote_name_buf (char **inbuf, size_t bufsize, char *name,
       quoted = false;
     }
 
-  if (needs_separator_protection)
-    for (char *p = buf; p < buf + len; p++)
-      if ((eolbyte && *p == '\n') || (format == with_commas && *p == ','))
-        *p = '?';
-
-  if (needs_further_quoting)
+  if (needs_further_quoting || needs_separator_protection)
     {
       if (MB_CUR_MAX > 1)
         {
@@ -4514,10 +4509,23 @@ quote_name_buf (char **inbuf, size_t bufsize, char *name,
           char const *plimit = buf + len;
           char *q = buf;
           displayed_width = 0;
+          bool width_valid = true;
 
           while (p < plimit)
-            switch (*p)
-              {
+            {
+              /* P points to the start of a (multi-byte) character here.  */
+              if (needs_separator_protection
+                  && ((eolbyte && *p == '\n')
+                      || (format == with_commas && *p == ',')))
+                {
+                  *q++ = '?';
+                  p++;
+                  displayed_width += 1;
+                  continue;
+                }
+
+              switch (*p)
+                {
                 case ' ': case '!': case '"': case '#': case '%':
                 case '&': case '\'': case '(': case ')': case '*':
                 case '+': case ',': case '-': case '.': case '/':
@@ -4544,8 +4552,9 @@ quote_name_buf (char **inbuf, size_t bufsize, char *name,
                   break;
                 default:
                   /* If we have a multibyte sequence, copy it until we
-                     reach its end, replacing each non-printable multibyte
-                     character with a single question mark.  */
+                     reach its end, replacing separators and, when
+                     requested, non-printable characters with question
+                     marks.  */
                   {
                     mbstate_t mbstate; mbszero (&mbstate);
                     do
@@ -4556,25 +4565,26 @@ quote_name_buf (char **inbuf, size_t bufsize, char *name,
 
                         bytes = mbrtoc32 (&wc, p, plimit - p, &mbstate);
 
-                        if (bytes == (size_t) -1)
+                        if (bytes == (size_t) -1
+                            || bytes == (size_t) -2)
                           {
-                            /* An invalid multibyte sequence was
-                               encountered.  Skip one input byte, and
-                               put a question mark.  */
-                            p++;
-                            *q++ = '?';
-                            displayed_width += 1;
-                            break;
-                          }
-
-                        if (bytes == (size_t) -2)
-                          {
-                            /* An incomplete multibyte character
-                               at the end.  Replace it entirely with
-                               a question mark.  */
-                            p = plimit;
-                            *q++ = '?';
-                            displayed_width += 1;
+                            /* An invalid or incomplete multibyte sequence
+                               was encountered.  */
+                            size_t bad_bytes = (bytes == (size_t) -1
+                                                ? 1 : plimit - p);
+                            if (needs_further_quoting)
+                              {
+                                p += bad_bytes;
+                                *q++ = '?';
+                                displayed_width += 1;
+                              }
+                            else
+                              {
+                                memmove (q, p, bad_bytes);
+                                q += bad_bytes;
+                                p += bad_bytes;
+                                width_valid = false;
+                              }
                             break;
                           }
 
@@ -4583,44 +4593,53 @@ quote_name_buf (char **inbuf, size_t bufsize, char *name,
                           bytes = 1;
 
                         w = c32width (wc);
-                        if (w >= 0)
+                        if (w < 0 && needs_further_quoting)
                           {
-                            /* A printable multibyte character.
-                               Keep it.  */
-                            for (; bytes > 0; --bytes)
-                              *q++ = *p++;
-                            displayed_width += w;
+                            /* Replace a nonprintable multibyte character.  */
+                            p += bytes;
+                            *q++ = '?';
+                            displayed_width += 1;
                           }
                         else
                           {
-                            /* An nonprintable multibyte character.
-                               Replace it entirely with a question
-                               mark.  */
+                            /* Keep this multibyte character.  */
+                            memmove (q, p, bytes);
+                            q += bytes;
                             p += bytes;
-                            *q++ = '?';
-                            displayed_width += 1;
+                            if (w < 0)
+                              width_valid = false;
+                            else
+                              displayed_width += w;
                           }
                       }
                     while (! mbsinit (&mbstate));
                   }
                   break;
-              }
+                }
+            }
 
           /* The buffer may have shrunk.  */
           len = q - buf;
+          if (! width_valid)
+            displayed_width = SIZE_MAX;
         }
       else
         {
           char *p = buf;
           char const *plimit = buf + len;
+          displayed_width = 0;
 
           while (p < plimit)
             {
-              if (! isprint (to_uchar (*p)))
+              if ((needs_separator_protection
+                   && ((eolbyte && *p == '\n')
+                       || (format == with_commas && *p == ',')))
+                  || (needs_further_quoting && ! isprint (to_uchar (*p))))
                 *p = '?';
+              if (isprint (to_uchar (*p)))
+                displayed_width++;
               p++;
             }
-          displayed_width = len;
         }
     }
   else if (width != NULL)
diff --git a/tests/ls/quoting-utf8.sh b/tests/ls/quoting-utf8.sh
index 8b15c38f5..2e40f3a0d 100755
--- a/tests/ls/quoting-utf8.sh
+++ b/tests/ls/quoting-utf8.sh
@@ -22,6 +22,15 @@ print_ver_ ls
 test "$(LC_ALL=en_US.UTF-8 locale charmap 2>/dev/null)" = UTF-8 ||
   skip_ 'en_US.UTF-8 locale not available'
 
+# Separator protection must preserve multibyte characters and unrelated
+# control bytes when they are requested literally.
+separator_name=$(printf 'multi\303\251,\001byte')
+touch "$separator_name" || framework_failure_
+printf 'multi\303\251?\001byte\n' > exp || framework_failure_
+LC_ALL=en_US.UTF-8 ls -m --show-control-chars --quoting-style=literal \
+  "$separator_name" > out || fail=1
+compare exp out || fail=1
+
 touch 'hello world' "it's" 'say "hi"' 'tab	here' || framework_failure_
 
 # A non-ASCII (C1) control character: U+0085 NEL = \xc2\x85.
-- 
2.55.0

Reply via email to