From: Pádraig Brady <P@draigBrady.com>
Date: Sat, 9 Apr 2022 14:46:52 +0000 (+0100)
Subject: cp,mv,install: avoid opening non directory destination
X-Git-Tag: v9.1~18
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=cc01b8a8f43bd1e02339322595f7a20e72a8c155;p=thirdparty%2Fcoreutils.git

cp,mv,install: avoid opening non directory destination

commit v9.0-66-ge2daa8f79 introduced an issue, for example
where cp could hang when overwriting a destination fifo,
when it would try to open() the fifo on systems
like Solaris 10 that didn't support the O_DIRECTORY flag.

This is still racy on such systems, but only in the
case where a directory is replaced by a fifo in
the small window between stat() and open().

* src/system.h (target_directory_operand): On systems without
O_DIRECTORY, ensure the file is a directory before attempting to open().
* tests/cp/special-f.sh: Protect cp with timeout(1),
as cp was seen to hang when trying to overwrite an existing fifo.
* NEWS: Mention the bug fix.
---

diff --git a/NEWS b/NEWS
index 85fd5be122..69feffcbc1 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,11 @@ GNU coreutils NEWS                                    -*- outline -*-
   and B is in some other file system.
   [bug introduced in coreutils-9.0]
 
+  cp, mv, and install, on older systems like Solaris 10 without O_DIRECTORY
+  support, avoid opening non directory destination operands.  Opening such
+  files can have side effects, like hanging with a fifo for example.
+  [bug introduced in coreutils-9.0]
+
   On macOS, fmt no longer corrupts multi-byte characters
   by misdetecting their component bytes as spaces.
   [This bug was present in "the beginning".]
diff --git a/src/system.h b/src/system.h
index 09498a172b..c24cb4dc39 100644
--- a/src/system.h
+++ b/src/system.h
@@ -136,13 +136,26 @@ target_directory_operand (char const *file)
   if (must_be_working_directory (file))
     return AT_FDCWD;
 
-  int fd = open (file, O_PATHSEARCH | O_DIRECTORY);
+  int fd = -1;
+  bool is_a_dir = false;
+  struct stat st;
+
+  /* On old systems like Solaris 10, check with stat first
+     lest we try to open a fifo for example and hang.  */
+  if (!O_DIRECTORY && stat (file, &st) == 0)
+    {
+      is_a_dir = !!S_ISDIR (st.st_mode);
+      if (! is_a_dir)
+        errno = ENOTDIR;
+    }
+
+  if (O_DIRECTORY || is_a_dir)
+    fd = open (file, O_PATHSEARCH | O_DIRECTORY);
 
   if (!O_DIRECTORY && 0 <= fd)
     {
-      /* On old systems like Solaris 10 that do not support O_DIRECTORY,
-         check by hand whether FILE is a directory.  */
-      struct stat st;
+      /* On old systems like Solaris 10 double check type,
+         to ensure we've opened a directory.  */
       int err;
       if (fstat (fd, &st) != 0 ? (err = errno, true)
           : !S_ISDIR (st.st_mode) && (err = ENOTDIR, true))
diff --git a/tests/cp/special-f.sh b/tests/cp/special-f.sh
index a3e7842fad..2c7bf5ea72 100755
--- a/tests/cp/special-f.sh
+++ b/tests/cp/special-f.sh
@@ -24,12 +24,10 @@ mkfifo_or_skip_ fifo
 
 touch e || framework-failure
 
-
-# Without -f, expect it to fail.
-cp -R fifo e || fail=1
-
-# With -f, it must succeed.
-cp -Rf fifo e || fail=1
-test -p fifo || fail=1
+for force in '' '-f'; do
+  # Second time through will need to unlink fifo e
+  timeout 10 cp -R $force fifo e || fail=1
+  test -p fifo || fail=1
+done
 
 Exit $fail