From: Michael Orlitzky Date: Mon, 26 Feb 2018 03:11:47 +0000 (-0500) Subject: Add safe_open() function to fix symlink traversal in non-terminal components. X-Git-Tag: v0.1.0~18 X-Git-Url: https://gitweb.michael.orlitzky.com/?p=apply-default-acl.git;a=commitdiff_plain;h=2dee89ff5a032f896d57a353c04525c506372669 Add safe_open() function to fix symlink traversal in non-terminal components. 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. --- diff --git a/src/apply-default-acl.c b/src/apply-default-acl.c index b7ef3a8..8979d1c 100644 --- a/src/apply-default-acl.c +++ b/src/apply-default-acl.c @@ -14,6 +14,7 @@ #include /* nftw() et al. */ #include #include /* basename(), dirname() */ +#include /* PATH_MAX */ #include #include #include @@ -33,6 +34,92 @@ #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 */