Naively ignore hard links to avoid security mishaps.

[apply-default-acl.git] / src / apply-default-acl.c

diff --git a/src/apply-default-acl.c b/src/apply-default-acl.c
index 465f70ad9794f55220f43abb8692d833238d2e4d..b89a732d9251556357dc9d52396aa657af80e15d 100644 (file)
--- a/src/apply-default-acl.c
+++ b/src/apply-default-acl.c
@@ -10,6 +10,7 @@
 #define _GNU_SOURCE
 
 #include <errno.h>
+#include <fcntl.h>  /* AT_FOO constants */
 #include <ftw.h>    /* nftw() et al. */
 #include <getopt.h>
 #include <libgen.h> /* dirname()     */
@@ -50,19 +51,44 @@ mode_t get_mode(const char* path) {
   }
 
   struct stat s;
-  int result = stat(path, &s);
+  int result = lstat(path, &s);
 
   if (result == 0) {
     return s.st_mode;
   }
   else {
-    /* errno will be set already by stat() */
+    /* errno will be set already by lstat() */
     return result;
   }
 }
 
 
 
+/**
+ * @brief Determine if the given path might refer to an (unsafe) hard link.
+ *
+ * @param path
+ *   The path to test.
+ *
+ * @return true if we are certain that @c path does not refer to a hard
+ *   link, and false otherwise. In case of error, false is returned,
+ *   because we are not sure that @c path is not a hard link.
+ */
+bool is_hardlink_safe(const char* path) {
+  if (path == NULL) {
+    return false;
+  }
+  struct stat s;
+  int result = lstat(path, &s);
+  if (result == 0) {
+    return (s.st_nlink == 1 || S_ISDIR(s.st_mode));
+  }
+  else {
+    return false;
+  }
+}
+
+
 /**
  * @brief Determine whether or not the given path is a regular file.
  *
@@ -77,7 +103,7 @@ bool is_regular_file(const char* path) {
   }
 
   struct stat s;
-  int result = stat(path, &s);
+  int result = lstat(path, &s);
   if (result == 0) {
     return S_ISREG(s.st_mode);
   }
@@ -88,6 +114,42 @@ bool is_regular_file(const char* path) {
 
 
 
+/**
+ * @brief Determine whether or not the given path is accessible.
+ *
+ * @param path
+ *   The path to test.
+ *
+ * @return true if @c path is accessible to the current effective
+ *   user/group, false otherwise.
+ */
+bool path_accessible(const char* path) {
+  if (path == NULL) {
+    return false;
+  }
+
+  /*  Test for access using the effective user and group rather than
+      the real one. */
+  int flags = AT_EACCESS;
+
+  /* Don't follow symlinks when checking for a path's existence,
+     since we won't follow them to set its ACLs either. */
+  flags |= AT_SYMLINK_NOFOLLOW;
+
+  /* If the path is relative, interpret it relative to the current
+     working directory (just like the access() system call). */
+  int result = faccessat(AT_FDCWD, path, F_OK, flags);
+
+  if (result == 0) {
+    return true;
+  }
+  else {
+    return false;
+  }
+}
+
+
+
 /**
  * @brief Determine whether or not the given path is a directory.
  *
@@ -102,7 +164,7 @@ bool is_directory(const char* path) {
   }
 
   struct stat s;
-  int result = stat(path, &s);
+  int result = lstat(path, &s);
   if (result == 0) {
     return S_ISDIR(s.st_mode);
   }
@@ -641,6 +703,32 @@ int apply_default_acl(const char* path, bool no_exec_mask) {
     return ACL_ERROR;
   }
 
+  /* Refuse to operate on hard links, which can be abused by an
+   * attacker to trick us into changing the ACL on a file we didn't
+   * intend to; namely the "target" of the hard link. To truly prevent
+   * that sort of mischief, we should be using file descriptors for
+   * the target and its parent directory. Then modulo a tiny race
+   * condition, we would be sure that "path" and "parent" don't change
+   * their nature between the time that we test them and when we
+   * utilize them. For contrast, the same attacker is free to replace
+   * "path" with a hard link after is_hardlink_safe() has returned
+   * "true" below.
+   *
+   * Unfortunately, our API is lacking in this area. For example,
+   * acl_set_fd() is only capable of setting the ACL_TYPE_ACCESS list,
+   * and not the ACL_TYPE_DEFAULT. Apparently the only way to operate
+   * on default ACLs is through the path name, which is inherently
+   * unreliable since the acl_*_file() calls themselves might follow
+   * links (both hard and symbolic).
+   *
+   * Some improvement could still be made by using descriptors where
+   * possible -- this would shrink the exploit window -- but for now
+   * we use a naive implementation that only keeps honest men honest.
+  */
+  if (!is_hardlink_safe(path)) {
+    return ACL_FAILURE;
+  }
+
   if (!is_regular_file(path) && !is_directory(path)) {
     return ACL_FAILURE;
   }
@@ -971,11 +1059,22 @@ int main(int argc, char* argv[]) {
     const char* target = argv[arg_index];
     bool reapp_result = false;
 
+    /* Make sure we can access the given path before we go out of our
+     * way to please it. Doing this check outside of
+     * apply_default_acl() lets us spit out a better error message for
+     * typos, too.
+     */
+    if (!path_accessible(target)) {
+      fprintf(stderr, "%s: %s: No such file or directory\n", argv[0], target);
+      result = EXIT_FAILURE;
+      continue;
+    }
+
     if (recursive) {
       reapp_result = apply_default_acl_recursive(target, no_exec_mask);
     }
     else {
-      /* It's either normal file, or we're not operating recursively. */
+      /* It's either a normal file, or we're not operating recursively. */
       reapp_result = apply_default_acl(target, no_exec_mask);
     }