]> gitweb.michael.orlitzky.com - apply-default-acl.git/commitdiff
Add safe_open() function to fix symlink traversal in non-terminal components.
authorMichael Orlitzky <michael@orlitzky.com>
Mon, 26 Feb 2018 03:11:47 +0000 (22:11 -0500)
committerMichael Orlitzky <michael@orlitzky.com>
Mon, 26 Feb 2018 19:10:29 +0000 (14:10 -0500)
The standard library provides lots of ways to avoid symlinks in the
"baz" component of "foo/bar/baz", but very few (i.e. zero) ways to
avoid them in the "bar" component. Of course, they're just as
dangerous in either place, so it would be cool if we could ignore
symlinks entirely.

This commit adds a safe_open() function, which looks just like open()
to the caller, but which starts at the root and calls openat() one
component at-a-time. Thus if you use O_NOFOLLOW, nobody can trick you
with an intermediate component: there are no intermediate components;
it works one at-a-time. This slows things down a bit, but not fatally.

src/apply-default-acl.c

index b7ef3a82c951f83e167c9dd00653402f7b74ef95..8979d1c56eb1955922d4b9705de9f4876781e3d2 100644 (file)
@@ -14,6 +14,7 @@
 #include <ftw.h>    /* nftw() et al. */
 #include <getopt.h>
 #include <libgen.h> /* basename(), dirname() */
+#include <limits.h> /* PATH_MAX */
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #define ACL_SUCCESS 1
 
 
+int safe_open_ex(int at_fd, char* pathname, int flags) {
+  if (pathname != NULL && 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;
+  }
+
+  /* Temporarily disable the slash, so that the subsequent call to
+     openat() opens only the next directory (and doesn't recurse). */
+   *firstslash = '\0';
+   int fd = safe_open_ex(at_fd, pathname, flags);
+
+   /* 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. */
+   int result = safe_open_ex(fd, firstslash+1, flags);
+   if (close(fd) == -1) {
+      perror("safe_open_ex (close)");
+      return -1;
+    }
+   return result;
+}
+
+
+int safe_open(const char* pathname, int flags) {
+  if (pathname == NULL || strlen(pathname) == 0 || pathname[0] == '\0') {
+    /* error? */
+    return -1;
+  }
+
+  char abspath[PATH_MAX];
+  int snprintf_result = 0;
+  if (strchr(pathname, '/') == pathname) {
+    /* pathname is already absolute; just copy it. */
+    snprintf_result = snprintf(abspath, PATH_MAX, "%s", pathname);
+  }
+  else {
+    /* Concatenate the current working directory and pathname into an
+     * absolute path. We use realpath() ONLY on the cwd part, and not
+     * on the pathname part, because realpath() resolves symlinks. And
+     * the whole point of all this crap is to avoid following symlinks
+     * in the pathname.
+     *
+     * Using realpath() on the cwd lets us operate on relative paths
+     * while we're sitting in a directory that happens to have a
+     * symlink in it; for example: cd /var/run && apply-default-acl foo.
+     */
+    char* cwd = get_current_dir_name();
+    if (cwd == NULL) {
+      perror("safe_open (get_current_dir_name)");
+      return -1;
+    }
+
+    char abs_cwd[PATH_MAX];
+    if (realpath(cwd, abs_cwd) == NULL) {
+      perror("safe_open (realpath)");
+      free(cwd);
+      return -1;
+    }
+    snprintf_result = snprintf(abspath, PATH_MAX, "%s/%s", abs_cwd, pathname);
+    free(cwd);
+  }
+  if (snprintf_result == -1 || snprintf_result > PATH_MAX) {
+    perror("safe_open (snprintf)");
+    return -1;
+  }
+
+  int fd = open("/", flags);
+  if (strcmp(abspath, "/") == 0) {
+    return fd;
+  }
+
+  int result = safe_open_ex(fd, abspath+1, flags);
+  if (close(fd) == -1) {
+    perror("safe_open (close)");
+    return -1;
+  }
+  return result;
+}
+
 
 
 /**
@@ -546,7 +633,7 @@ int apply_default_acl(const char* path,
   }
   char* parent = dirname(path_copy);
 
-  fd = open(path, O_NOFOLLOW);
+  fd = safe_open(path, O_NOFOLLOW);
   if (fd == -1) {
     if (errno == ELOOP) {
       result = ACL_FAILURE; /* hit a symlink */