Guard against /proc/<pid>/cwd attack.

This exploit against safepath_check was reported by
Travis Ormandy in the comp.unix.programmer Usenet
newsgroup on July 29th (UTC), message ID
jkgrb9FhdslU1 <at> mid.individual.net.

On Linux, if you change to some directory and
run the "su" program, the operating system will
spin up a process whose /proc/<pid> directory is
root-owned, and not writable (thus safe-looking)
and contains a cwd symlink pointing to that
directory.

Regular users cannot follow this symlink, but
root can, which makes it an attack vector.

There eare more unsafe links under /proc;
this will be addressed in another commit.

* safepath.c (simplify_path): New static function,
for removing "..", "." and empty components
from a path without filesystem access. When
we are checking absolute paths for unsafe patterns
we must use simplified paths, otherwise these
components can be used to evade the matches we
are looking for. However, we cannot then do all
of our safety checks on a simplified path because
a simplified path can be completely safe, whereas
the original isn't: e.g. foo/unsafe/../bar simplifies
to foo/bar.
(abs_path_check): New static function.
Here we will place all knowledge about special
paths that are possible attack vectors, starting
with this ill-considered /proc/<pid>/cwd.
(safepath_check): If the input path is absolute,
check it with abs_path_check. Also, if a symlink
target is an absolute path, check it also.

1 files changed, 109 insertions, 3 deletions

diff --git a/safepath.c b/safepath.c
index 587b07f..7bb5315 100644
--- a/safepath.c
+++ b/safepath.c
@@ -125,6 +125,99 @@ static int tamper_proof(const struct stat *st)
   }
 }
 
+/*
+ * Get rid of .. and . components, without filesystem access.
+ * Returns malloced string.
+ */
+static char *simplify_path(const char *ipath)
+{
+  char *opath = malloc(strlen(ipath) + 1);
+
+  if (opath != 0) {
+    size_t ipos = 0, opos = 0;
+
+    opath[ipos] = 0;
+
+    if (ipath[ipos] == '/') {
+      opath[opos++] = ipath[ipos++];
+      opath[ipos] = 0;
+    }
+
+    for (;;) {
+      size_t complen = strcspn(ipath + ipos, "/");
+
+      if (complen == 2 && strncmp(ipath + ipos, "..", 2) == 0 && opos > 0) {
+        int ppos = opos - 1;
+        size_t pcomplen = 0;
+
+        while (ppos > 0 && opath[ppos - 1] != '/') {
+          ppos--;
+          pcomplen++;
+        }
+
+        if (pcomplen > 0 && (pcomplen != 2 ||
+                             strncmp(ipath + ppos, "..", 2) != 0))
+        {
+          opos = ppos;
+          opath[opos] = 0;
+          goto nextcomp;
+        } else {
+          goto copy;
+        }
+      } else if ((complen == 1 && ipath[ipos] == '.') ||
+                 complen == 0)
+      {
+        goto nextcomp;
+      }
+
+    copy:
+      strncat(opath + opos, ipath + ipos, complen);
+      opos += complen;
+      if (ipath[ipos + complen]) {
+        strcat(opath + opos, "/");
+        opos++;
+      }
+    nextcomp:
+      ipos += complen;
+      if (ipath[ipos] == 0)
+        break;
+      ipos++;
+    }
+  }
+
+  return opath;
+}
+
+/*
+ * Checks for some known system paths that can be attack vectors.
+ */
+static int abs_path_check(const char *abspath)
+{
+  /* The /proc/<pid>/cwd symlink cannot be trusted by root, because an
+   * unprivileged user can use the "su" executable to point that anywhere.
+   * Non-root cannot access that symlink, and so is safe from it.
+   */
+  char *sabspath = simplify_path(abspath);
+
+  if (geteuid() == 0) {
+    const char *proc = "/proc/";
+    size_t proclen = strlen(proc);
+
+    if (strncmp(sabspath, proc, proclen) == 0) {
+      const char *pid = sabspath + proclen;
+      size_t pidlen = strspn(pid, "0123456789");
+
+      if (pid[pidlen] == '/' || pid[pidlen] == 0) {
+        free(sabspath);
+        return 0;
+      }
+    }
+  }
+
+  free(sabspath);
+  return 1;
+}
+
 static int safepath_err(int eno)
 {
   switch (eno) {
@@ -178,10 +271,11 @@ static void set_errno(int spres)
 int safepath_check(const char *name)
 {
   struct stat st;
-  const char *start = (*name == '/') ? "/" : ".";
-  size_t pos = (*name == '/') ? 1 : 0;
+  int abs = (*name == '/');
+  const char *start = abs ? "/" : ".";
+  size_t pos = abs ? 1 : 0;
   char *copy;
-  int ret = SAFEPATH_OK, count = 0, root_checked = (*name == '/');
+  int ret = SAFEPATH_OK, count = 0, root_checked = abs;
 
   /* empty name is invalid */
   if (*name == 0) {
@@ -189,6 +283,12 @@ int safepath_check(const char *name)
     goto out;
   }
 
+  /* check absolute path for known vulnerabilities. */
+  if (abs && !abs_path_check(name)) {
+    ret = SAFEPATH_UNSAFE;
+    goto out;
+  }
+
   /* check starting directory */
   if (stat(start, &st) < 0) {
     ret = safepath_err(errno);
@@ -277,6 +377,12 @@ int safepath_check(const char *name)
        * Either way it's string grafting.
        */
       if (link[0] == '/') {
+        /* Check absolute path for known vulnerabilities. */
+        if (!abs_path_check(link)) {
+          ret = SAFEPATH_UNSAFE;
+          goto free_out;
+
+        }
         /* We have to check the root directory, if we have
          * not done so before.
          */