X-Git-Url: http://gitweb.michael.orlitzky.com/?a=blobdiff_plain;f=src%2Fapply-default-acl.c;h=3cf3060a115e2be50f75822d13319e5362684a82;hb=f0970d1193ebddbfcd2ba9523a0f956aed2bd4cc;hp=8979d1c56eb1955922d4b9705de9f4876781e3d2;hpb=2dee89ff5a032f896d57a353c04525c506372669;p=apply-default-acl.git

diff --git a/src/apply-default-acl.c b/src/apply-default-acl.c
index 8979d1c..3cf3060 100644
--- a/src/apply-default-acl.c
+++ b/src/apply-default-acl.c
@@ -33,7 +33,30 @@
 #define ACL_FAILURE 0
 #define ACL_SUCCESS 1
 
+/* Even though most other library functions reliably return -1 for
+ * error, it feels a little wrong to re-use the ACL_ERROR constant.
+ */
+#define CLOSE_ERROR -1
+#define NFTW_ERROR -1
+#define OPEN_ERROR -1
+#define SNPRINTF_ERROR -1
+#define STAT_ERROR -1
+
 
+/**
+ * @brief The recursive portion of the @c safe_open function, used to
+ *   open a file descriptor in a symlink-safe way when combined with
+ *   the @c O_NOFOLLOW flag.
+ *
+ * @param at_fd
+ *   A file descriptor relative to which @c pathname will be opened.
+ *
+ * @param pathname
+ *   The path to the file/directory/whatever whose descriptor you want.
+ *
+ * @return a file descriptor for @c pathname if everything goes well,
+ *   and @c OPEN_ERROR if not.
+ */
 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. */
@@ -51,23 +74,53 @@ int safe_open_ex(int at_fd, char* pathname, int flags) {
      openat() opens only the next directory (and doesn't recurse). */
    *firstslash = '\0';
    int fd = safe_open_ex(at_fd, pathname, flags);
+   if (fd == OPEN_ERROR) {
+     if (errno != ELOOP) {
+       /* 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. */
    int result = safe_open_ex(fd, firstslash+1, flags);
-   if (close(fd) == -1) {
+   if (close(fd) == CLOSE_ERROR) {
       perror("safe_open_ex (close)");
-      return -1;
+      return OPEN_ERROR;
     }
    return result;
 }
 
 
+/**
+ * @brief A version of @c open that is completely symlink-safe when
+ *   used with the @c O_NOFOLLOW flag.
+ *
+ * The @c openat function exists to ensure that you can anchor one
+ * path to a particular directory while opening it; however, if you
+ * open "b/c/d" relative to "/a", then even the @c openat function will
+ * still follow symlinks in the "b" component. This can be exploited
+ * by an attacker to make you open the wrong path.
+ *
+ * To avoid that problem, this function uses a recursive
+ * implementation that opens every path from the root, one level at a
+ * time. So "a" is opened relative to "/", and then "b" is opened
+ * relative to "/a", and then "c" is opened relative to "/a/b",
+ * etc. When the @c O_NOFOLLOW flag is used, this approach ensures
+ * that no symlinks in any component are followed.
+ *
+ * @param pathname
+ *   The path to the file/directory/whatever whose descriptor you want.
+ *
+ * @return a file descriptor for @c pathname if everything goes well,
+ *   and @c OPEN_ERROR if not.
+ */
 int safe_open(const char* pathname, int flags) {
   if (pathname == NULL || strlen(pathname) == 0 || pathname[0] == '\0') {
     /* error? */
-    return -1;
+    return OPEN_ERROR;
   }
 
   char abspath[PATH_MAX];
@@ -90,21 +143,21 @@ int safe_open(const char* pathname, int flags) {
     char* cwd = get_current_dir_name();
     if (cwd == NULL) {
       perror("safe_open (get_current_dir_name)");
-      return -1;
+      return OPEN_ERROR;
     }
 
     char abs_cwd[PATH_MAX];
     if (realpath(cwd, abs_cwd) == NULL) {
       perror("safe_open (realpath)");
       free(cwd);
-      return -1;
+      return OPEN_ERROR;
     }
     snprintf_result = snprintf(abspath, PATH_MAX, "%s/%s", abs_cwd, pathname);
     free(cwd);
   }
-  if (snprintf_result == -1 || snprintf_result > PATH_MAX) {
+  if (snprintf_result == SNPRINTF_ERROR || snprintf_result > PATH_MAX) {
     perror("safe_open (snprintf)");
-    return -1;
+    return OPEN_ERROR;
   }
 
   int fd = open("/", flags);
@@ -113,9 +166,9 @@ int safe_open(const char* pathname, int flags) {
   }
 
   int result = safe_open_ex(fd, abspath+1, flags);
-  if (close(fd) == -1) {
+  if (close(fd) == CLOSE_ERROR) {
     perror("safe_open (close)");
-    return -1;
+    return OPEN_ERROR;
   }
   return result;
 }
@@ -634,7 +687,7 @@ int apply_default_acl(const char* path,
   char* parent = dirname(path_copy);
 
   fd = safe_open(path, O_NOFOLLOW);
-  if (fd == -1) {
+  if (fd == OPEN_ERROR) {
     if (errno == ELOOP) {
       result = ACL_FAILURE; /* hit a symlink */
       goto cleanup;
@@ -659,7 +712,7 @@ int apply_default_acl(const char* path,
   */
   if (sp == NULL) {
     struct stat s;
-    if (fstat(fd, &s) == -1) {
+    if (fstat(fd, &s) == STAT_ERROR) {
       perror("apply_default_acl (fstat)");
       goto cleanup;
     }
@@ -820,7 +873,7 @@ int apply_default_acl(const char* path,
   if (defacl != (acl_t)NULL) {
     acl_free(defacl);
   }
-  if (fd >= 0 && close(fd) == -1) {
+  if (fd >= 0 && close(fd) == CLOSE_ERROR) {
     perror("apply_default_acl (close)");
     result = ACL_ERROR;
   }
@@ -941,11 +994,11 @@ bool apply_default_acl_recursive(const char *target, bool no_exec_mask) {
     return true;
   }
 
-  /* nftw will return -1 on error, or if the supplied function
+  /* nftw will return NFTW_ERROR on error, or if the supplied function
    * (apply_default_acl_nftw) returns a non-zero result, nftw will
    * return that.
    */
-  if (nftw_result == -1) {
+  if (nftw_result == NFTW_ERROR) {
     perror("apply_default_acl_recursive (nftw)");
   }