Hi,
The resolvpath() function in kern_pledge.c is used for wl_paths
(whitelisted-paths).
It permits to obtain a full resolved path (absolute and chroot-agnostic)
from a path supplied by user in order to check it against list of
whitelisted paths.
The order of the operations inside the function is wrong in the case of
the user supplies a relative path including lot of ../../../../
Currently the order is (schematically) the following:
- prepend cwd if relative
- prepend chroot prefix if chrooted
- canonize the path (reduce /../, /./ or // inside the string)
The following patch reorders it to:
- prepend cwd if relative
- canonize the path
- prepend chroot prefix if chrooted
so chroot prefix isn't touched any more by any "../../../../" in the
user supplied path.
Please note that this code path is currently unreachable as wl_paths is
disabled. The impact would be invalid whitelisting (bypassing), but the
operations after would *not* be altered and the program would not gain
any privileges (only information leaks as ENOENT wouldn't be returned as
it should).
Comments ? OK ?
--
Sebastien Marie
Index: kern/kern_pledge.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.155
diff -u -p -r1.155 kern_pledge.c
--- kern/kern_pledge.c 13 Mar 2016 18:40:52 -0000 1.155
+++ kern/kern_pledge.c 14 Mar 2016 05:42:39 -0000
@@ -1625,43 +1625,12 @@ resolvpath(struct proc *p,
char **resolved, size_t *resolvedlen)
{
int error;
- char *fullpath = NULL, *rawcanopath = NULL, *canopath = NULL;
- size_t fullpathlen, rawcanopathlen, canopathlen;
+ char *abspath = NULL, *canopath = NULL, *fullpath = NULL;
+ size_t abspathlen, canopathlen, fullpathlen, canopathlen_exact;
- /* 1. get rdir if chrooted */
- if (p->p_fd->fd_rdir != NULL) {
- if (*rdir == NULL) {
- char *rawrdir, *bp, *bpend;
- size_t rawrdirlen = MAXPATHLEN * 4;
-
- rawrdir = malloc(rawrdirlen, M_TEMP, M_WAITOK);
- bp = &rawrdir[rawrdirlen];
- bpend = bp;
- *(--bp) = '\0';
-
- error = vfs_getcwd_common(p->p_fd->fd_rdir,
- rootvnode, &bp, rawrdir, rawrdirlen/2,
- GETCWD_CHECK_ACCESS, p);
- if (error) {
- free(rawrdir, M_TEMP, rawrdirlen);
- goto out;
- }
-
- /* NUL is included */
- *rdirlen = (bpend - bp);
- *rdir = malloc(*rdirlen, M_TEMP, M_WAITOK);
- memcpy(*rdir, bp, *rdirlen);
-
- free(rawrdir, M_TEMP, rawrdirlen);
- }
- } else {
- if (*rdir == NULL)
- *rdirlen = 0; /* ensure rdirlen value is initialized
*/
- }
-
- /* 2. resolv: path -> fullpath */
+ /* 1. get an absolute path (inside any chroot) : path -> abspath */
if (path[0] != '/') {
- /* path is relative: prepend cwd (rootvnode based) */
+ /* path is relative: prepend cwd */
/* get cwd first (if needed) */
if (*cwd == NULL) {
@@ -1674,7 +1643,7 @@ resolvpath(struct proc *p,
*(--bp) = '\0';
error = vfs_getcwd_common(p->p_fd->fd_cdir,
- rootvnode, &bp, rawcwd, rawcwdlen/2,
+ NULL, &bp, rawcwd, rawcwdlen/2,
GETCWD_CHECK_ACCESS, p);
if (error) {
free(rawcwd, M_TEMP, rawcwdlen);
@@ -1690,44 +1659,84 @@ resolvpath(struct proc *p,
}
/* NUL included in *cwdlen and pathlen */
- fullpathlen = *cwdlen + pathlen;
- fullpath = malloc(fullpathlen, M_TEMP, M_WAITOK);
- snprintf(fullpath, fullpathlen, "%s/%s", *cwd, path);
-
- } else if (p->p_fd->fd_rdir) {
- /* path is absolute and we are chrooted : prepend *rdir */
-
- /* NUL included in *rdirlen and pathlen (and no '/' between
them) */
- fullpathlen = *rdirlen + pathlen - 1;
- fullpath = malloc(fullpathlen, M_TEMP, M_WAITOK);
- snprintf(fullpath, fullpathlen, "%s%s", *rdir, path);
+ abspathlen = *cwdlen + pathlen;
+ abspath = malloc(abspathlen, M_TEMP, M_WAITOK);
+ snprintf(abspath, abspathlen, "%s/%s", *cwd, path);
} else {
/* path is absolute */
- fullpathlen = pathlen;
- fullpath = malloc(fullpathlen, M_TEMP, M_WAITOK);
- memcpy(fullpath, path, pathlen);
+ abspathlen = pathlen;
+ abspath = malloc(abspathlen, M_TEMP, M_WAITOK);
+ memcpy(abspath, path, pathlen);
}
- /* 3. canonization: fullpath -> rawcanopath */
- rawcanopathlen = fullpathlen;
- rawcanopath = malloc(rawcanopathlen, M_TEMP, M_WAITOK);
- error = canonpath(fullpath, rawcanopath, rawcanopathlen);
+ /* 2. canonization: abspath -> canopath */
+ canopathlen = abspathlen;
+ canopath = malloc(canopathlen, M_TEMP, M_WAITOK);
+ error = canonpath(abspath, canopath, canopathlen);
+
+ /* free abspath now as we don't need it after */
+ free(abspath, M_TEMP, abspathlen);
+
+ /* error in canonpath() call (should not happen, but keep safe) */
if (error != 0)
goto out;
- /* 4. output a fresh allocated path: rawcanopath -> canopath */
- canopathlen = strlen(rawcanopath) + 1; /* NUL */
- canopath = malloc(canopathlen, M_TEMP, M_WAITOK);
- memcpy(canopath, rawcanopath, canopathlen);
+ /* check the canopath size */
+ canopathlen_exact = strlen(canopath) + 1;
+ if (canopathlen_exact > MAXPATHLEN) {
+ error = ENAMETOOLONG;
+ goto out;
+ }
+
+ /* 3. preprend *rdir if chrooted : canonpath -> fullpath */
+ if (p->p_fd->fd_rdir != NULL) {
+ if (*rdir == NULL) {
+ char *rawrdir, *bp, *bpend;
+ size_t rawrdirlen = MAXPATHLEN * 4;
+
+ rawrdir = malloc(rawrdirlen, M_TEMP, M_WAITOK);
+ bp = &rawrdir[rawrdirlen];
+ bpend = bp;
+ *(--bp) = '\0';
+
+ error = vfs_getcwd_common(p->p_fd->fd_rdir,
+ rootvnode, &bp, rawrdir, rawrdirlen/2,
+ GETCWD_CHECK_ACCESS, p);
+ if (error) {
+ free(rawrdir, M_TEMP, rawrdirlen);
+ goto out;
+ }
+
+ /* NUL is included */
+ *rdirlen = (bpend - bp);
+ *rdir = malloc(*rdirlen, M_TEMP, M_WAITOK);
+ memcpy(*rdir, bp, *rdirlen);
+
+ free(rawrdir, M_TEMP, rawrdirlen);
+ }
+
+ /*
+ * NUL is included in *rdirlen and canopathlen_exact.
+ * doesn't add "/" between them, as canopath is absolute.
+ */
+ fullpathlen = *rdirlen + canopathlen_exact - 1;
+ fullpath = malloc(fullpathlen, M_TEMP, M_WAITOK);
+ snprintf(fullpath, fullpathlen, "%s%s", *rdir, canopath);
+
+ } else {
+ /* not chrooted: only reduce canopath to exact length */
+ fullpathlen = canopathlen_exact;
+ fullpath = malloc(fullpathlen, M_TEMP, M_WAITOK);
+ memcpy(fullpath, canopath, fullpathlen);
+ }
- *resolvedlen = canopathlen;
- *resolved = canopath;
+ *resolvedlen = fullpathlen;
+ *resolved = fullpath;
out:
- free(rawcanopath, M_TEMP, rawcanopathlen);
- free(fullpath, M_TEMP, fullpathlen);
+ free(canopath, M_TEMP, canopathlen);
if (error != 0)
- free(canopath, M_TEMP, canopathlen);
+ free(fullpath, M_TEMP, fullpathlen);
return error;
}
