cksum: use more defensive escaping for --check

cksum --check is often the first interaction
users have with possibly untrusted downloads, so we should try
to be as defensive as possible when processing it.

Specifically we currently only escape \n characters in file names
presented in checksum files being parsed with cksum --check.
This gives some possibilty of dumping arbitrary data to the terminal
when checking downloads from an untrusted source.
This change gives these advantages:

  1. Avoids dumping arbitrary data to vulnerable terminals
  2. Avoids visual deception with ansi codes hiding checksum failures
  3. More secure if users copy and paste file names from --check output
  4. Simplifies programmatic parsing

Note this changes programmatic parsing, but given the original
format was so awkward to parse, I expect that's extremely rare.
I was not able to find example in the wild at least.
To parse the new format from from shell, you can do something like:

  cksum -c checksums | while IFS= read -r line; do
    case $line in
      *': FAILED')
        filename=$(eval "printf '%s' ${line%: FAILED}")
        cp -v "$filename" /quarantine
        ;;
    esac
  done

This change also slightly reduces the size of the sum(1) utility.
This change also apples to md5sum, sha*sum, and b2sum.

* src/cksum.c (digest_check): Call quotef() instead of
cksum(1) specific quoting.
* tests/cksum/md5sum-bsd.sh: Adjust accordingly.
* doc/coreutils.texi (cksum general options): Describe the
shell quoting used for problematic file names.
* NEWS: Mention the change in behavior.
Reported by: Aaron Rainbolt

diff --git a/NEWS b/NEWS
index 340115eccfc9bfd3894566fd3ecd53221fa59b2e..5302c1ccd951aa543853402625b77c136dac147c 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,12 @@ GNU coreutils NEWS                                    -*- outline -*-
   'date --date' now parses dot delimited dd.mm.yy format common in Europe.
   This is in addition to the already supported mm/dd/yy and yy-mm-dd formats.
 
+** Changes in behavior
+
+  'cksum --check' now uses shell quoting when required, to more robustly
+  escape file names output in diagnostics.
+  This also affects md5sum, sha*sum, and b2sum.
+
 ** Improvements
 
   'df --local' recognises more file system types as remote.

diff --git a/doc/coreutils.texi b/doc/coreutils.texi
index f3d8c81ca5d13ed41ee5e6f8ad2ca96f78c049b5..c9b67c339c8378f457ece79ebe3c09f84088cf85 100644 (file)
--- a/doc/coreutils.texi
+++ b/doc/coreutils.texi
@@ -4130,7 +4130,11 @@ By default, for each valid line, one line is written to standard
 output indicating whether the named file passed the test.
 After all checks have been performed, if there were any failures,
 a warning is issued to standard error.
-Use the @option{--status} option to inhibit that output.
+If file names contain problematic characters,
+they will be quoted in a way suitable for POSIX-compatible shells,
+so that any file name is represented unambiguously and safely.
+
+Use the @option{--status} option to inhibit the output described above.
 If any listed file cannot be opened or read, if any valid line has
 a checksum inconsistent with the associated file, or if no valid
 line is found, @command{cksum} exits with nonzero status.  Otherwise,

diff --git a/src/cksum.c b/src/cksum.c
index 14119c6a1c35ceab831b699389ca1741ba2375d8..8bc83cf93b65968a7643c83fc6b1fa829323a24a 100644 (file)
--- a/src/cksum.c
+++ b/src/cksum.c
@@ -705,6 +705,7 @@ or equivalent standalone program.\
   exit (status);
 }
 
+#if !HASH_ALGO_SUM
 /* Given a string S, return TRUE if it contains problematic characters
    that need escaping.  Note we escape '\' itself to provide some forward
    compat to introduce escaping of other characters.  */
@@ -716,6 +717,7 @@ problematic_chars (char const *s)
   idx_t length = strcspn (s, "\\\n\r");
   return s[length] != '\0';
 }
+#endif
 
 /* Given a file name, S of length S_LEN, that is not NUL-terminated,
    modify it in place, performing the equivalent of this sed substitution:
@@ -1128,6 +1130,7 @@ split_3 (char *s, idx_t s_len,
   return true;
 }
 
+#if !HASH_ALGO_SUM
 /* If ESCAPE is true, then translate each:
    NEWLINE byte to the string, "\\n",
    CARRIAGE RETURN byte to the string, "\\r",
@@ -1164,6 +1167,7 @@ print_filename (char const *file, bool escape)
       file++;
     }
 }
+#endif
 
 /* An interface to the function, DIGEST_STREAM.
    Operate on FILENAME (it may be "-").
@@ -1444,7 +1448,6 @@ digest_check (char const *checkfile_name)
         {
           bool ok;
           bool missing;
-          bool needs_escape = ! status_only && problematic_chars (filename);
 
           properly_formatted_lines = true;
 
@@ -1455,12 +1458,7 @@ digest_check (char const *checkfile_name)
             {
               ++n_open_or_read_failures;
               if (!status_only)
-                {
-                  if (needs_escape)
-                    putchar ('\\');
-                  print_filename (filename, needs_escape);
-                  printf (": %s\n", _("FAILED open or read"));
-                }
+                printf ("%s: %s\n", quotef (filename),_("FAILED open or read"));
             }
           else if (ignore_missing && missing)
             {
@@ -1486,11 +1484,7 @@ digest_check (char const *checkfile_name)
               if (!status_only)
                 {
                   if (! match || ! quiet)
-                    {
-                      if (needs_escape)
-                        putchar ('\\');
-                      print_filename (filename, needs_escape);
-                    }
+                    fputs (quotef (filename), stdout);
 
                   if (! match)
                     printf (": %s\n", _("FAILED"));

diff --git a/tests/cksum/md5sum-bsd.sh b/tests/cksum/md5sum-bsd.sh
index 7f526bf00e2d1ce4fef7b7c00762d7c1ef82e8b0..7859fbbf5787ff92eb1cd0c089c734e03d1947db 100755 (executable)
--- a/tests/cksum/md5sum-bsd.sh
+++ b/tests/cksum/md5sum-bsd.sh
@@ -82,8 +82,8 @@ if echo '' > 'backslash\is\not\dir\sep'; then
     md5sum --tag "$i" >> check.md5 || fail=1
   done
   md5sum --strict -c check.md5 > out || fail=1
-  printf '%s: OK\n' '\a\\b' '\a\\' '\\\a' '\a\nb' "a${t}b" > exp ||
-    framework_failure_
+  printf '%s: OK\n' "'a\\b'" "'a\\'" "'\\a'" \
+                    "'a'\$'\\n''b'" "'a'\$'\\t''b'" > exp || framework_failure_
   compare exp out || fail=1