From: Michael Orlitzky <michael@orlitzky.com>
Date: Wed, 28 Feb 2018 14:55:25 +0000 (-0500)
Subject: libadacl.c: use O_PATH in safe_open() for added safety.
X-Git-Tag: v0.1.4~4
X-Git-Url: http://gitweb.michael.orlitzky.com/?p=apply-default-acl.git;a=commitdiff_plain;h=f9e643a6aaf9d03fd6c36eb0ad953f2563a7c017

libadacl.c: use O_PATH in safe_open() for added safety.
---

diff --git a/src/libadacl.c b/src/libadacl.c
index 5402a32..246f30a 100644
--- a/src/libadacl.c
+++ b/src/libadacl.c
@@ -8,7 +8,7 @@
 /* Enables get_current_dir_name() in unistd.h */
 #define _GNU_SOURCE
 
-#include <errno.h>    /* ELOOP, EINVAL, etc. */
+#include <errno.h>    /* EINVAL, ELOOP, ENOTDIR, etc. */
 #include <fcntl.h>    /* openat() */
 #include <libgen.h>   /* basename(), dirname() */
 #include <limits.h>   /* PATH_MAX */
@@ -40,9 +40,6 @@
  *   open a file descriptor in a symlink-safe way when combined with
  *   the @c O_NOFOLLOW flag.
  *
- * The @c O_PATH flag is not used because we want to fail upon
- * encountering any symlinks.
- *
  * @param at_fd
  *   A file descriptor relative to which @c pathname will be opened.
  *
@@ -62,33 +59,33 @@ int safe_open_ex(int at_fd, char* pathname, int flags) {
     return OPEN_ERROR;
   }
 
-  if (strlen(pathname) == 0) {
-    /* Oops, went one level to deep with nothing to do. */
-    return at_fd;
-  }
-
   char* firstslash = strchr(pathname, '/');
   if (firstslash == NULL) {
     /* No more slashes, this is the base case. */
-    int r = openat(at_fd, pathname, flags);
-    return r;
+    return openat(at_fd, pathname, flags);
+  }
+  else if (firstslash[1] == '\0') {
+    /* The first slash is the last character; ensure that we open
+       a directory. */
+    firstslash[0] = '\0';
+    return openat(at_fd, pathname, flags | O_DIRECTORY);
   }
 
-  /* Temporarily disable the slash, so that the subsequent call to
-     openat() opens only the next directory (and doesn't recurse). */
+  /* The first slash exists and isn't the last character in the path,
+     so we can split the path wherever that first slash lies and
+     recurse. */
    *firstslash = '\0';
-   int fd = safe_open_ex(at_fd, pathname, flags);
+   int fd = openat(at_fd, pathname, flags | O_DIRECTORY | O_PATH);
    if (fd == OPEN_ERROR) {
-     if (errno != ELOOP) {
+     if (errno != ENOTDIR) {
        /* Don't output anything if we ignore a symlink */
        perror("safe_open_ex (safe_open_ex)");
      }
      return OPEN_ERROR;
    }
 
-   /* The ++ is safe because there needs to be at least a null byte
-      after the first slash, even if it's the last real character in
-      the string. */
+   /* The +1 is safe because there needs to be at least one character
+      after the first slash (we checked this above). */
    int result = safe_open_ex(fd, firstslash+1, flags);
    if (close(fd) == CLOSE_ERROR) {
       perror("safe_open_ex (close)");
@@ -168,7 +165,14 @@ int safe_open(const char* pathname, int flags) {
     return OPEN_ERROR;
   }
 
-  int fd = open("/", flags);
+  int fd = 0;
+  if (strcmp(abspath, "/") == 0) {
+    fd = open("/", flags | O_DIRECTORY);
+  }
+  else {
+    /* Use O_PATH for some added safety if "/" is not our target */
+    fd = open("/", flags | O_DIRECTORY | O_PATH);
+  }
   if (fd == OPEN_ERROR) {
     perror("safe_open (open)");
     return OPEN_ERROR;
@@ -687,8 +691,10 @@ int apply_default_acl_ex(const char* path,
 
   fd = safe_open(path, O_NOFOLLOW);
   if (fd == OPEN_ERROR) {
-    if (errno == ELOOP) {
-      result = ACL_FAILURE; /* hit a symlink */
+    if (errno == ELOOP || errno == ENOTDIR) {
+      /* We hit a symlink, either in the last path component (ELOOP)
+	 or higher up (ENOTDIR). */
+      result = ACL_FAILURE;
       goto cleanup;
     }
     else {