Merge branch 'wip/smcv/issue8' into 'master'

fdio: Don't change ownership of symlinks if not requested

Closes #8

See merge request GNOME/libglnx!83

glnx-fdio.c

@@ -745,9 +745,12 @@ copy_symlink_at (int                   src_dfd,
         return FALSE;
     }
 
-  if (TEMP_FAILURE_RETRY (fchownat (fd, "", src_stbuf->st_uid, src_stbuf->st_gid,
-                                    AT_EMPTY_PATH)) != 0)
-    return glnx_throw_errno_prefix (error, "fchownat");
+  if (!(copyflags & GLNX_FILE_COPY_NOCHOWN))
+    {
+      if (TEMP_FAILURE_RETRY (fchownat (fd, "", src_stbuf->st_uid, src_stbuf->st_gid,
+                                        AT_EMPTY_PATH)) != 0)
+        return glnx_throw_errno_prefix (error, "fchownat");
+    }
 
   return TRUE;
 }

tests/test-libglnx-fdio.c

@@ -355,11 +355,17 @@ test_filecopy (void)
   g_assert (S_ISREG (stbuf.st_mode));
 }
 
+/* uid/gid 5 is normally 'tty', e.g. <https://systemd.io/UIDS-GIDS/>,
+ * which is a reasonably harmless one to use */
+const uid_t not_root_uid = 5;
+const gid_t not_root_gid = 5;
+
 static void
 test_copy_symlink (void)
 {
   _GLNX_TEST_DECLARE_ERROR(local_error, error);
   struct stat stbuf;
+  g_autofree char *expected_target = NULL;
   g_autofree char *target = NULL;
 
   if (symlinkat ("sometarget", AT_FDCWD, "srclink") < 0)
@@ -377,6 +383,84 @@ test_copy_symlink (void)
   if (!target)
     return;
   g_assert_cmpstr (target, ==, "sometarget");
+
+  /* /dev/stderr is a convenient example of a symlink that will often exist,
+   * and is often owned by root. This means that unprivileged users will
+   * not be able to set the copy's ownership to equal the original's. */
+  expected_target = glnx_readlinkat_malloc (AT_FDCWD, "/dev/stderr", NULL, error);
+
+  if (expected_target != NULL)
+    {
+      g_autofree char *actual_target = NULL;
+
+      if (!glnx_file_copy_at (AT_FDCWD, "/dev/stderr", NULL,
+                              AT_FDCWD, "stderr",
+                              GLNX_FILE_COPY_NOCHOWN | GLNX_FILE_COPY_NOXATTRS,
+                              NULL, error))
+        return;
+
+      actual_target = glnx_readlinkat_malloc (AT_FDCWD, "stderr", NULL, error);
+
+      if (!actual_target)
+        return;
+
+      g_assert_cmpstr (actual_target, ==, expected_target);
+    }
+  else
+    {
+      g_test_message ("Not testing /dev/stderr: %s", local_error->message);
+      g_clear_error (&local_error);
+    }
+
+  /* If we're running the test as root, we expect that copying /dev/stderr
+   * would have succeeded even if we incorrectly changed its ownership.
+   * However, if we're root, we can construct a symlink owned by someone else
+   * on-demand, and use that. */
+  if (symlinkat ("sometarget", AT_FDCWD, "owned-by-other") < 0)
+    return (void) glnx_throw_errno_prefix (error, "symlinkat");
+
+  if (geteuid () != 0)
+    {
+      g_test_message ("Not testing symlink owned by another user: not root");
+    }
+  else if (lchown ("owned-by-other", not_root_uid, not_root_gid) < 0)
+    {
+      g_test_message ("Not testing symlink owned by another user: %s",
+                      g_strerror (errno));
+    }
+  else
+    {
+      g_autofree char *actual_target = NULL;
+
+      if (!glnx_fstatat (AT_FDCWD, "owned-by-other", &stbuf,
+                         AT_SYMLINK_NOFOLLOW, error))
+        return;
+
+      g_assert_cmpint (stbuf.st_uid, ==, not_root_uid);
+      g_assert_cmpint (stbuf.st_gid, ==, not_root_gid);
+
+      if (!glnx_file_copy_at (AT_FDCWD, "owned-by-other", NULL,
+                              AT_FDCWD, "owned-by-other-copy",
+                              GLNX_FILE_COPY_NOCHOWN | GLNX_FILE_COPY_NOXATTRS,
+                              NULL, error))
+        return;
+
+      if (!glnx_fstatat (AT_FDCWD, "owned-by-other-copy", &stbuf,
+                         AT_SYMLINK_NOFOLLOW, error))
+        return;
+
+      g_assert_true (S_ISLNK (stbuf.st_mode));
+      g_assert_cmpint (stbuf.st_uid, !=, not_root_uid);
+      g_assert_cmpint (stbuf.st_gid, !=, not_root_gid);
+
+      actual_target = glnx_readlinkat_malloc (AT_FDCWD, "owned-by-other-copy",
+                                              NULL, error);
+
+      if (!actual_target)
+        return;
+
+      g_assert_cmpstr (actual_target, ==, "sometarget");
+    }
 }
 
 static void