Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(206)

Unified Diff: src/trusted/service_runtime/sys_filename.c

Issue 1211173002: add restricted filesystem access to sel_ldr Base URL: https://chromium.googlesource.com/native_client/src/native_client.git@master
Patch Set: Created 5 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 trivia
jtolds 2015/06/26 22:07:11 currently working through these comments, but i ju
+ */
+}
+
+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 sh
+ */
+ 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 syml
+ * 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;
« src/trusted/service_runtime/sys_fdio.c ('K') | « src/trusted/service_runtime/sys_fdio.c ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698