add restricted filesystem access to sel_ldr

src/trusted/service_runtime/sys_filename.c

Index: src/trusted/service_runtime/sys_filename.c
diff --git a/src/trusted/service_runtime/sys_filename.c b/src/trusted/service_runtime/sys_filename.c
index 41edc2385bf1e707f48f3337d16225ffb1a9164f..61aa888cd44dc9220892303676665d93c31ee919 100644
--- a/src/trusted/service_runtime/sys_filename.c
+++ b/src/trusted/service_runtime/sys_filename.c
@@ -44,6 +44,49 @@ static uint32_t CopyPathFromUser(struct NaClApp *nap,
   return 0;
 }
 
+static uint32_t CopyHostPathFromUser(struct NaClApp *nap,
+                                     char           *dest,
+                                     size_t         num_bytes,
+                                     uintptr_t      src) {
+  /*
+   * The purpose of this function is to not only copy the path bytes from the
+   * NaClApp, but to make sure the path is properly sanitized and has the
+   * NaClRootFolder prefixed, if provided.
+   * Start and see if there even is a root.
+   */
+  if (NaClRootFolder == NULL) {
+    return CopyPathFromUser(nap, dest, num_bytes, src);
+  }
+  /*
+   * Make sure we have room for the trailing null byte and an actual path.
+   */
+  if (NaClRootFolderLen + 1 >= num_bytes) {
+    return -NACL_ABI_ENAMETOOLONG;
+  }
+  /*
+   * Okay, add the root folder to dest (which will have a trailing slash
+   * already), then copy the path in after.
+   */
+  memcpy(dest, NaClRootFolder, NaClRootFolderLen);
+  return CopyPathFromUser(nap,
+                          dest + NaClRootFolderLen,
+                          num_bytes - NaClRootFolderLen,
+                          src);
+  /*
+   * TODO(jtolds): make sure we sanitize the path we just got so it has no ".."
+   *  path elements.

[jtolds, 2015/06/25 23:05:05]

does this even need to happen?

[Mark Seaborn, 2015/06/25 23:55:09]

If you don't remove ".." elements, isn't it trivial to read outside of
NaClRootFolder?

[jtolds, 2015/06/26 22:07:11]

currently working through these comments, but i just didn't know if path
sanitization happened at some higher level or something.

+   */
+}
+
+int NaClCopyHostPathOutToUser(struct NaClApp  *nap,
+                              uintptr_t       dst_usr_addr,
+                              char            *path) {
+  /*
+   * TODO(jtolds): remove NaClRootFolder prefix and fail if it doesn't match

[jtolds, 2015/06/25 23:05:05]

this function is only used to get the CWD. what should happen if the CWD is not
inside the untrusted root? should we chdir before starting to reduce that
chance?

+   */
+  return NaClCopyOutToUser(nap, dst_usr_addr, path, strlen(path) + 1);
+}
+
 int32_t NaClSysOpen(struct NaClAppThread  *natp,
                     uint32_t              pathname,
                     int                   flags,
@@ -58,11 +101,11 @@ int32_t NaClSysOpen(struct NaClAppThread  *natp,
           "0x%08"NACL_PRIx32", 0x%x, 0x%x)\n",
           (uintptr_t) natp, pathname, flags, mode);
 
-  if (!NaClAclBypassChecks) {
+  if (!NaClAclBypassChecks && NaClRootFolder == NULL) {
     return -NACL_ABI_EACCES;
   }
 
-  retval = CopyPathFromUser(nap, path, sizeof path, (uintptr_t) pathname);
+  retval = CopyHostPathFromUser(nap, path, sizeof path, (uintptr_t) pathname);
   if (0 != retval)
     goto cleanup;
 
@@ -166,11 +209,11 @@ int32_t NaClSysStat(struct NaClAppThread  *natp,
           ("Entered NaClSysStat(0x%08"NACL_PRIxPTR", 0x%08"NACL_PRIx32","
            " 0x%08"NACL_PRIx32")\n"), (uintptr_t) natp, pathname, nasp);
 
-  if (!NaClAclBypassChecks) {
+  if (!NaClAclBypassChecks && NaClRootFolder == NULL) {
     return -NACL_ABI_EACCES;
   }
 
-  retval = CopyPathFromUser(nap, path, sizeof path, pathname);
+  retval = CopyHostPathFromUser(nap, path, sizeof path, pathname);
   if (0 != retval)
     goto cleanup;
 
@@ -197,12 +240,12 @@ int32_t NaClSysMkdir(struct NaClAppThread *natp,
   char           path[NACL_CONFIG_PATH_MAX];
   int32_t        retval = -NACL_ABI_EINVAL;
 
-  if (!NaClAclBypassChecks) {
+  if (!NaClAclBypassChecks && NaClRootFolder == NULL) {
     retval = -NACL_ABI_EACCES;
     goto cleanup;
   }
 
-  retval = CopyPathFromUser(nap, path, sizeof path, pathname);
+  retval = CopyHostPathFromUser(nap, path, sizeof path, pathname);
   if (0 != retval)
     goto cleanup;
 
@@ -217,12 +260,12 @@ int32_t NaClSysRmdir(struct NaClAppThread *natp,
   char           path[NACL_CONFIG_PATH_MAX];
   int32_t        retval = -NACL_ABI_EINVAL;
 
-  if (!NaClAclBypassChecks) {
+  if (!NaClAclBypassChecks && NaClRootFolder == NULL) {
     retval = -NACL_ABI_EACCES;
     goto cleanup;
   }
 
-  retval = CopyPathFromUser(nap, path, sizeof path, pathname);
+  retval = CopyHostPathFromUser(nap, path, sizeof path, pathname);
   if (0 != retval)
     goto cleanup;
 
@@ -237,12 +280,12 @@ int32_t NaClSysChdir(struct NaClAppThread *natp,
   char           path[NACL_CONFIG_PATH_MAX];
   int32_t        retval = -NACL_ABI_EINVAL;
 
-  if (!NaClAclBypassChecks) {
+  if (!NaClAclBypassChecks && NaClRootFolder == NULL) {
     retval = -NACL_ABI_EACCES;
     goto cleanup;
   }
 
-  retval = CopyPathFromUser(nap, path, sizeof path, pathname);
+  retval = CopyHostPathFromUser(nap, path, sizeof path, pathname);
   if (0 != retval)
     goto cleanup;
 
@@ -258,7 +301,7 @@ int32_t NaClSysGetcwd(struct NaClAppThread *natp,
   int32_t        retval = -NACL_ABI_EINVAL;
   char           path[NACL_CONFIG_PATH_MAX];
 
-  if (!NaClAclBypassChecks) {
+  if (!NaClAclBypassChecks && NaClRootFolder == NULL) {
     retval = -NACL_ABI_EACCES;
     goto cleanup;
   }
@@ -270,7 +313,7 @@ int32_t NaClSysGetcwd(struct NaClAppThread *natp,
   if (retval != 0)
     goto cleanup;
 
-  if (!NaClCopyOutToUser(nap, buffer, &path, strlen(path) + 1))
+  if (!NaClCopyHostPathOutToUser(nap, buffer, &path[0]))
     retval = -NACL_ABI_EFAULT;
 
 cleanup:
@@ -283,12 +326,12 @@ int32_t NaClSysUnlink(struct NaClAppThread *natp,
   char           path[NACL_CONFIG_PATH_MAX];
   int32_t        retval = -NACL_ABI_EINVAL;
 
-  if (!NaClAclBypassChecks) {
+  if (!NaClAclBypassChecks && NaClRootFolder == NULL) {
     retval = -NACL_ABI_EACCES;
     goto cleanup;
   }
 
-  retval = CopyPathFromUser(nap, path, sizeof path, pathname);
+  retval = CopyHostPathFromUser(nap, path, sizeof path, pathname);
   if (0 != retval)
     goto cleanup;
 
@@ -306,10 +349,10 @@ int32_t NaClSysTruncate(struct NaClAppThread *natp,
   int32_t        retval = -NACL_ABI_EINVAL;
   nacl_abi_off_t length;
 
-  if (!NaClAclBypassChecks)
+  if (!NaClAclBypassChecks && NaClRootFolder == NULL)
     return -NACL_ABI_EACCES;
 
-  retval = CopyPathFromUser(nap, path, sizeof path, pathname);
+  retval = CopyHostPathFromUser(nap, path, sizeof path, pathname);
   if (0 != retval)
     return retval;
 
@@ -334,27 +377,51 @@ int32_t NaClSysLstat(struct NaClAppThread  *natp,
           ("Entered NaClSysLstat(0x%08"NACL_PRIxPTR", 0x%08"NACL_PRIx32","
            " 0x%08"NACL_PRIx32")\n"), (uintptr_t) natp, pathname, nasp);
 
-  if (!NaClAclBypassChecks) {
-    return -NACL_ABI_EACCES;
-  }
+  if (NaClRootFolder != NULL) {
+    retval = CopyHostPathFromUser(nap, path, sizeof path, pathname);
+    if (0 != retval)
+      return retval;
 
-  retval = CopyPathFromUser(nap, path, sizeof path, pathname);
-  if (0 != retval)
+    /*
+     * Mounted root folders don't support symlinks yet, so pretend they don't
+     * exist, and all symlinks are actually whatever they point to. CAREFUL,
+     * this means having symlinks that point outside of your mounted root

[Mark Seaborn, 2015/06/25 23:55:09]

Furthermore, note that if you have a relative symlink whose destination contains
a "..", it might start out pointing to within the sandbox filesystem, but
rename() or link() could be used to move it so that it points outside.

This is the kind of detail that can be covered in a file in documentation/.

+     * folder can allow chroot jail breaking!
+     * For this reason, symlink creation is also disabled.
+     */
+    retval = NaClHostDescStat(path, &stbuf);
+    if (0 == retval) {
+      struct nacl_abi_stat abi_stbuf;
+
+      retval = NaClAbiStatHostDescStatXlateCtor(&abi_stbuf, &stbuf);
+      if (!NaClCopyOutToUser(nap, nasp, &abi_stbuf, sizeof abi_stbuf)) {
+        return -NACL_ABI_EFAULT;
+      }
+    }
     return retval;
 
-  /*
-   * Perform a host stat.
-   */
-  retval = NaClHostDescLstat(path, &stbuf);
-  if (0 == retval) {
-    struct nacl_abi_stat abi_stbuf;
+  } else if (NaClAclBypassChecks) {
+    retval = CopyPathFromUser(nap, path, sizeof path, pathname);
+    if (0 != retval)
+      return retval;
 
-    retval = NaClAbiStatHostDescStatXlateCtor(&abi_stbuf, &stbuf);
-    if (!NaClCopyOutToUser(nap, nasp, &abi_stbuf, sizeof abi_stbuf)) {
-      return -NACL_ABI_EFAULT;
+    /*
+     * Perform a host lstat directly
+     */
+    retval = NaClHostDescLstat(path, &stbuf);
+    if (0 == retval) {
+      struct nacl_abi_stat abi_stbuf;
+
+      retval = NaClAbiStatHostDescStatXlateCtor(&abi_stbuf, &stbuf);
+      if (!NaClCopyOutToUser(nap, nasp, &abi_stbuf, sizeof abi_stbuf)) {
+        return -NACL_ABI_EFAULT;
+      }
     }
+    return retval;
+
+  } else {
+    return -NACL_ABI_EACCES;
   }
-  return retval;
 }
 
 int32_t NaClSysLink(struct NaClAppThread *natp,
@@ -365,14 +432,14 @@ int32_t NaClSysLink(struct NaClAppThread *natp,
   char           newpath[NACL_CONFIG_PATH_MAX];
   int32_t        retval = -NACL_ABI_EINVAL;
 
-  if (!NaClAclBypassChecks)
+  if (!NaClAclBypassChecks && NaClRootFolder == NULL)
     return -NACL_ABI_EACCES;
 
-  retval = CopyPathFromUser(nap, oldpath, sizeof oldpath, oldname);
+  retval = CopyHostPathFromUser(nap, oldpath, sizeof oldpath, oldname);
   if (0 != retval)
     return retval;
 
-  retval = CopyPathFromUser(nap, newpath, sizeof newpath, newname);
+  retval = CopyHostPathFromUser(nap, newpath, sizeof newpath, newname);
   if (0 != retval)
     return retval;
 
@@ -387,14 +454,14 @@ int32_t NaClSysRename(struct NaClAppThread *natp,
   char           newpath[NACL_CONFIG_PATH_MAX];
   int32_t        retval = -NACL_ABI_EINVAL;
 
-  if (!NaClAclBypassChecks)
+  if (!NaClAclBypassChecks && NaClRootFolder == NULL)
     return -NACL_ABI_EACCES;
 
-  retval = CopyPathFromUser(nap, oldpath, sizeof oldpath, oldname);
+  retval = CopyHostPathFromUser(nap, oldpath, sizeof oldpath, oldname);
   if (0 != retval)
     return retval;
 
-  retval = CopyPathFromUser(nap, newpath, sizeof newpath, newname);
+  retval = CopyHostPathFromUser(nap, newpath, sizeof newpath, newname);
   if (0 != retval)
     return retval;
 
@@ -409,6 +476,15 @@ int32_t NaClSysSymlink(struct NaClAppThread *natp,
   char           newpath[NACL_CONFIG_PATH_MAX];
   int32_t        retval = -NACL_ABI_EINVAL;
 
+  if (NaClRootFolder != NULL) {
+    /*
+     * Mounted root folders don't support symlinks yet, so pretend they don't
+     * exist and can't be created. See also lstat and readlink.
+     * Could be convinced to do ENOSYS instead of EPERM.
+     */
+    return -NACL_ABI_EPERM;
+  }
+
   if (!NaClAclBypassChecks)
     return -NACL_ABI_EACCES;
 
@@ -430,10 +506,10 @@ int32_t NaClSysChmod(struct NaClAppThread *natp,
   char           pathname[NACL_CONFIG_PATH_MAX];
   int32_t        retval = -NACL_ABI_EINVAL;
 
-  if (!NaClAclBypassChecks)
+  if (!NaClAclBypassChecks && NaClRootFolder == NULL)
     return -NACL_ABI_EACCES;
 
-  retval = CopyPathFromUser(nap, pathname, sizeof pathname, path);
+  retval = CopyHostPathFromUser(nap, pathname, sizeof pathname, path);
   if (0 != retval)
     return retval;
 
@@ -447,7 +523,7 @@ int32_t NaClSysAccess(struct NaClAppThread *natp,
   char           pathname[NACL_CONFIG_PATH_MAX];
   int32_t        retval = -NACL_ABI_EINVAL;
 
-  if (!NaClAclBypassChecks)
+  if (!NaClAclBypassChecks && NaClRootFolder == NULL)
     return -NACL_ABI_EACCES;
 
   /*
@@ -457,7 +533,7 @@ int32_t NaClSysAccess(struct NaClAppThread *natp,
       && (amode & ~(NACL_ABI_R_OK | NACL_ABI_W_OK | NACL_ABI_X_OK)) != 0)
     return -NACL_ABI_EINVAL;
 
-  retval = CopyPathFromUser(nap, pathname, sizeof pathname, path);
+  retval = CopyHostPathFromUser(nap, pathname, sizeof pathname, path);
   if (0 != retval)
     return retval;
 
@@ -476,6 +552,14 @@ int32_t NaClSysReadlink(struct NaClAppThread *natp,
   int32_t        retval = -NACL_ABI_EINVAL;
   uint32_t       result_size;
 
+  if (NaClRootFolder != NULL) {
+    /* We're going to return an error, but perhaps in this case we should
+     * try and find out if the file even exists, so we can return ENOENT first,
+     * and then return EINVAL otherwise? Dunno.
+     */
+    return -NACL_ABI_ENOSYS;
+  }
+
   if (!NaClAclBypassChecks)
     return -NACL_ABI_EACCES;
 
@@ -518,10 +602,10 @@ int32_t NaClSysUtimes(struct NaClAppThread *natp,
   char           pathname[NACL_CONFIG_PATH_MAX];
   int32_t        retval = -NACL_ABI_EINVAL;
 
-  if (!NaClAclBypassChecks)
+  if (!NaClAclBypassChecks && NaClRootFolder == NULL)
     return -NACL_ABI_EACCES;
 
-  retval = CopyPathFromUser(nap, pathname, sizeof pathname, path);
+  retval = CopyHostPathFromUser(nap, pathname, sizeof pathname, path);
   if (0 != retval)
     return retval;