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 19505282c2acda17960281e966a9d347a5bc1611..b89a732d9251556357dc9d52396aa657af80e15d 100644 (file)
--- a/src/apply-default-acl.c
+++ b/src/apply-default-acl.c
@@ -64,6 +64,31 @@ mode_t get_mode(const char* path) {
 
 
 
+/**
+ * @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.
  *
@@ -678,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;
   }