Merge branch 'master' into feature/jq

CHANGELOG.md

@@ -6,6 +6,8 @@ Blocks of changes will separated by version increments.
 
 ## **[Unreleased]**
 
+- [#463](https://github.com/wasmerio/wasmer/pull/463) Fix bug in WASI path_open allowing one level above preopened dir to be accessed
+- [#461](https://github.com/wasmerio/wasmer/pull/461) Prevent passing negative lengths in various places in the runtime C API
 - [#459](https://github.com/wasmerio/wasmer/pull/459) Add monotonic and real time clocks for wasi on windows
 - [#447](https://github.com/wasmerio/wasmer/pull/447) Add trace macro (`--features trace`) for more verbose debug statements
 - [#451](https://github.com/wasmerio/wasmer/pull/451) Add `--mapdir=src:dest` flag to rename host directories in the guest context

lib/runtime-c-api/src/import.rs

@@ -8,7 +8,7 @@ use crate::{
     value::wasmer_value_tag,
     wasmer_byte_array, wasmer_result_t,
 };
-use libc::{c_int, uint32_t};
+use libc::{c_uint, uint32_t};
 use std::{ffi::c_void, ptr, slice, sync::Arc};
 use wasmer_runtime::Module;
 use wasmer_runtime_core::{
@@ -154,11 +154,11 @@ pub extern "C" fn wasmer_import_descriptors_destroy(
 #[no_mangle]
 pub unsafe extern "C" fn wasmer_import_descriptors_len(
     exports: *mut wasmer_import_descriptors_t,
-) -> c_int {
+) -> c_uint {
     if exports.is_null() {
         return 0;
     }
-    (*(exports as *mut NamedImportDescriptors)).0.len() as c_int
+    (*(exports as *mut NamedImportDescriptors)).0.len() as c_uint
 }
 
 /// Gets import descriptor by index
@@ -166,7 +166,7 @@ pub unsafe extern "C" fn wasmer_import_descriptors_len(
 #[no_mangle]
 pub unsafe extern "C" fn wasmer_import_descriptors_get(
     import_descriptors: *mut wasmer_import_descriptors_t,
-    idx: c_int,
+    idx: c_uint,
 ) -> *mut wasmer_import_descriptor_t {
     if import_descriptors.is_null() {
         return ptr::null_mut();
@@ -244,9 +244,9 @@ pub unsafe extern "C" fn wasmer_import_func_params_arity(
 pub unsafe extern "C" fn wasmer_import_func_new(
     func: extern "C" fn(data: *mut c_void),
     params: *const wasmer_value_tag,
-    params_len: c_int,
+    params_len: c_uint,
     returns: *const wasmer_value_tag,
-    returns_len: c_int,
+    returns_len: c_uint,
 ) -> *mut wasmer_import_func_t {
     let params: &[wasmer_value_tag] = slice::from_raw_parts(params, params_len as usize);
     let params: Vec<Type> = params.iter().cloned().map(|x| x.into()).collect();
@@ -272,7 +272,7 @@ pub unsafe extern "C" fn wasmer_import_func_new(
 pub unsafe extern "C" fn wasmer_import_func_params(
     func: *const wasmer_import_func_t,
     params: *mut wasmer_value_tag,
-    params_len: c_int,
+    params_len: c_uint,
 ) -> wasmer_result_t {
     let export = &*(func as *const Export);
     if let Export::Function { ref signature, .. } = *export {
@@ -301,7 +301,7 @@ pub unsafe extern "C" fn wasmer_import_func_params(
 pub unsafe extern "C" fn wasmer_import_func_returns(
     func: *const wasmer_import_func_t,
     returns: *mut wasmer_value_tag,
-    returns_len: c_int,
+    returns_len: c_uint,
 ) -> wasmer_result_t {
     let export = &*(func as *const Export);
     if let Export::Function { ref signature, .. } = *export {

lib/runtime-c-api/wasmer.h

@@ -321,12 +321,12 @@ void wasmer_import_descriptors_destroy(wasmer_import_descriptors_t *import_descr
  * Gets import descriptor by index
  */
 wasmer_import_descriptor_t *wasmer_import_descriptors_get(wasmer_import_descriptors_t *import_descriptors,
-                                                          int idx);
+                                                          unsigned int idx);
 
 /**
  * Gets the length of the import descriptors
  */
-int wasmer_import_descriptors_len(wasmer_import_descriptors_t *exports);
+unsigned int wasmer_import_descriptors_len(wasmer_import_descriptors_t *exports);
 
 /**
  * Frees memory for the given Func
@@ -339,9 +339,9 @@ void wasmer_import_func_destroy(wasmer_import_func_t *func);
  */
 wasmer_import_func_t *wasmer_import_func_new(void (*func)(void *data),
                                              const wasmer_value_tag *params,
-                                             int params_len,
+                                             unsigned int params_len,
                                              const wasmer_value_tag *returns,
-                                             int returns_len);
+                                             unsigned int returns_len);
 
 /**
  * Sets the params buffer to the parameter types of the given wasmer_import_func_t
@@ -351,7 +351,7 @@ wasmer_import_func_t *wasmer_import_func_new(void (*func)(void *data),
  */
 wasmer_result_t wasmer_import_func_params(const wasmer_import_func_t *func,
                                           wasmer_value_tag *params,
-                                          int params_len);
+                                          unsigned int params_len);
 
 /**
  * Sets the result parameter to the arity of the params of the wasmer_import_func_t
@@ -369,7 +369,7 @@ wasmer_result_t wasmer_import_func_params_arity(const wasmer_import_func_t *func
  */
 wasmer_result_t wasmer_import_func_returns(const wasmer_import_func_t *func,
                                            wasmer_value_tag *returns,
-                                           int returns_len);
+                                           unsigned int returns_len);
 
 /**
  * Sets the result parameter to the arity of the returns of the wasmer_import_func_t

lib/runtime-c-api/wasmer.hh

@@ -4,6 +4,7 @@
 #include <cstdarg>
 #include <cstdint>
 #include <cstdlib>
+#include <new>
 
 enum class wasmer_import_export_kind : uint32_t {
   WASM_FUNCTION,
@@ -260,10 +261,10 @@ void wasmer_import_descriptors_destroy(wasmer_import_descriptors_t *import_descr
 
 /// Gets import descriptor by index
 wasmer_import_descriptor_t *wasmer_import_descriptors_get(wasmer_import_descriptors_t *import_descriptors,
-                                                          int idx);
+                                                          unsigned int idx);
 
 /// Gets the length of the import descriptors
-int wasmer_import_descriptors_len(wasmer_import_descriptors_t *exports);
+unsigned int wasmer_import_descriptors_len(wasmer_import_descriptors_t *exports);
 
 /// Frees memory for the given Func
 void wasmer_import_func_destroy(wasmer_import_func_t *func);
@@ -272,9 +273,9 @@ void wasmer_import_func_destroy(wasmer_import_func_t *func);
 /// The caller owns the object and should call `wasmer_import_func_destroy` to free it.
 wasmer_import_func_t *wasmer_import_func_new(void (*func)(void *data),
                                              const wasmer_value_tag *params,
-                                             int params_len,
+                                             unsigned int params_len,
                                              const wasmer_value_tag *returns,
-                                             int returns_len);
+                                             unsigned int returns_len);
 
 /// Sets the params buffer to the parameter types of the given wasmer_import_func_t
 /// Returns `wasmer_result_t::WASMER_OK` upon success.
@@ -282,7 +283,7 @@ wasmer_import_func_t *wasmer_import_func_new(void (*func)(void *data),
 /// and `wasmer_last_error_message` to get an error message.
 wasmer_result_t wasmer_import_func_params(const wasmer_import_func_t *func,
                                           wasmer_value_tag *params,
-                                          int params_len);
+                                          unsigned int params_len);
 
 /// Sets the result parameter to the arity of the params of the wasmer_import_func_t
 /// Returns `wasmer_result_t::WASMER_OK` upon success.
@@ -296,7 +297,7 @@ wasmer_result_t wasmer_import_func_params_arity(const wasmer_import_func_t *func
 /// and `wasmer_last_error_message` to get an error message.
 wasmer_result_t wasmer_import_func_returns(const wasmer_import_func_t *func,
                                            wasmer_value_tag *returns,
-                                           int returns_len);
+                                           unsigned int returns_len);
 
 /// Sets the result parameter to the arity of the returns of the wasmer_import_func_t
 /// Returns `wasmer_result_t::WASMER_OK` upon success.

lib/wasi/src/syscalls/mod.rs

@@ -1463,97 +1463,117 @@ pub fn path_open(
         "Looking for file {} in directory {:#?}",
         file_name, cumulative_path
     );
+
     cumulative_path.push(file_name);
     let file_path = cumulative_path;
 
-    let out_fd = if let Kind::Dir { entries, .. } = &mut state.fs.inodes[cur_dir_inode].kind {
-        if let Some(child) = entries.get(file_name).cloned() {
-            let child_inode_val = &state.fs.inodes[child];
-            // early return based on flags
-            if o_flags & __WASI_O_EXCL != 0 {
-                return __WASI_EEXIST;
-            }
-            if o_flags & __WASI_O_DIRECTORY != 0 {
-                match &child_inode_val.kind {
-                    Kind::Dir { .. } => (),
-                    Kind::Symlink { .. } => {
-                        unimplemented!("Symlinks not yet supported in path_open")
-                    }
-                    _ => return __WASI_ENOTDIR,
-                }
-            }
-            // do logic on child
-            wasi_try!(state
-                .fs
-                .create_fd(fs_rights_base, fs_rights_inheriting, fs_flags, child))
-        } else {
-            debug!("Attempting to load file from host system");
-            let file_metadata = file_path.metadata();
-            // if entry does not exist in parent directory, try to lazily
-            // load it; possibly creating or truncating it if flags set
-            let kind = if file_metadata.is_ok() && file_metadata.unwrap().is_dir() {
-                // special dir logic
-                Kind::Dir {
-                    parent: Some(cur_dir_inode),
-                    path: file_path.clone(),
-                    entries: Default::default(),
-                }
+    let out_fd = if let Kind::Dir {
+        entries, parent, ..
+    } = &mut state.fs.inodes[cur_dir_inode].kind
+    {
+        // short circuit logic if attempting to get parent
+        if file_name == ".." {
+            if let Some(p) = parent {
+                let parent_inode = *p;
+                wasi_try!(state.fs.create_fd(
+                    fs_rights_base,
+                    fs_rights_inheriting,
+                    fs_flags,
+                    parent_inode
+                ))
             } else {
-                // file is not a dir
-                let real_opened_file = {
-                    let mut open_options = std::fs::OpenOptions::new();
-                    let open_options = open_options.read(true);
-                    let open_options = if fs_rights_base & __WASI_RIGHT_FD_WRITE != 0 {
-                        open_options.write(true)
-                    } else {
-                        open_options
-                    };
-                    let open_options = if o_flags & __WASI_O_CREAT != 0 {
-                        debug!(
-                            "File {:?} may be created when opened if it does not exist",
-                            &file_path
-                        );
-                        open_options.create(true)
-                    } else {
-                        open_options
-                    };
-                    let open_options = if o_flags & __WASI_O_TRUNC != 0 {
-                        debug!("File {:?} will be truncated when opened", &file_path);
-                        open_options.truncate(true)
-                    } else {
-                        open_options
-                    };
-                    debug!("Opening host file {:?}", &file_path);
-                    let real_open_file =
-                        wasi_try!(open_options.open(&file_path).map_err(|_| __WASI_EIO));
-
-                    real_open_file
-                };
-                Kind::File {
-                    handle: WasiFile::HostFile(real_opened_file),
-                }
-            };
-
-            // record lazily loaded or newly created fd
-            let new_inode = state.fs.inodes.insert(InodeVal {
-                stat: wasi_try!(get_stat_for_kind(&kind).ok_or(__WASI_EIO)),
-                is_preopened: false,
-                name: file_name.clone(),
-                kind,
-            });
-
-            // reborrow to insert entry
-            if let Kind::Dir { entries, .. } = &mut state.fs.inodes[working_dir.inode].kind {
-                entries.insert(file_name.clone(), new_inode);
+                return __WASI_EACCES;
             }
-            let new_fd = wasi_try!(state.fs.create_fd(
-                fs_rights_base,
-                fs_rights_inheriting,
-                fs_flags,
-                new_inode,
-            ));
+        } else {
+            if let Some(child) = entries.get(file_name).cloned() {
+                let child_inode_val = &state.fs.inodes[child];
+                // early return based on flags
+                if o_flags & __WASI_O_EXCL != 0 {
+                    return __WASI_EEXIST;
+                }
+                if o_flags & __WASI_O_DIRECTORY != 0 {
+                    match &child_inode_val.kind {
+                        Kind::Dir { .. } => (),
+                        Kind::Symlink { .. } => {
+                            unimplemented!("Symlinks not yet supported in path_open")
+                        }
+                        _ => return __WASI_ENOTDIR,
+                    }
+                }
+                // do logic on child
+                wasi_try!(state
+                    .fs
+                    .create_fd(fs_rights_base, fs_rights_inheriting, fs_flags, child))
+            } else {
+                debug!("Attempting to load file from host system");
 
-            new_fd
+                let file_metadata = file_path.metadata();
+                // if entry does not exist in parent directory, try to lazily
+                // load it; possibly creating or truncating it if flags set
+                let kind = if file_metadata.is_ok() && file_metadata.unwrap().is_dir() {
+                    // special dir logic
+                    Kind::Dir {
+                        parent: Some(cur_dir_inode),
+                        path: file_path.clone(),
+                        entries: Default::default(),
+                    }
+                } else {
+                    // file is not a dir
+                    let real_opened_file = {
+                        let mut open_options = std::fs::OpenOptions::new();
+                        let open_options = open_options.read(true);
+                        let open_options = if fs_rights_base & __WASI_RIGHT_FD_WRITE != 0 {
+                            open_options.write(true)
+                        } else {
+                            open_options
+                        };
+                        let open_options = if o_flags & __WASI_O_CREAT != 0 {
+                            debug!(
+                                "File {:?} may be created when opened if it does not exist",
+                                &file_path
+                            );
+                            open_options.create(true)
+                        } else {
+                            open_options
+                        };
+                        let open_options = if o_flags & __WASI_O_TRUNC != 0 {
+                            debug!("File {:?} will be truncated when opened", &file_path);
+                            open_options.truncate(true)
+                        } else {
+                            open_options
+                        };
+                        debug!("Opening host file {:?}", &file_path);
+                        let real_open_file =
+                            wasi_try!(open_options.open(&file_path).map_err(|_| __WASI_EIO));
+
+                        real_open_file
+                    };
+                    Kind::File {
+                        handle: WasiFile::HostFile(real_opened_file),
+                    }
+                };
+
+                // record lazily loaded or newly created fd
+                let new_inode = state.fs.inodes.insert(InodeVal {
+                    stat: wasi_try!(get_stat_for_kind(&kind).ok_or(__WASI_EIO)),
+                    is_preopened: false,
+                    name: file_name.clone(),
+                    kind,
+                });
+
+                // reborrow to insert entry
+                if let Kind::Dir { entries, .. } = &mut state.fs.inodes[working_dir.inode].kind {
+                    entries.insert(file_name.clone(), new_inode);
+                }
+                let new_fd = wasi_try!(state.fs.create_fd(
+                    fs_rights_base,
+                    fs_rights_inheriting,
+                    fs_flags,
+                    new_inode,
+                ));
+
+                new_fd
+            }
         }
     } else {
         // working_dir did not match on Kind::Dir

lib/wasi/tests/wasitests/fs_sandbox_test.rs

@@ -0,0 +1,9 @@
+#[test]
+fn test_fs_sandbox_test() {
+    assert_wasi_output!(
+        "../../wasitests/fs_sandbox_test.wasm",
+        "fs_sandbox_test",
+        vec![],
+        "../../wasitests/fs_sandbox_test.out"
+    );
+}

lib/wasi/tests/wasitests/mod.rs

@@ -7,6 +7,7 @@
 mod _common;
 mod create_dir;
 mod file_metadata;
+mod fs_sandbox_test;
 mod hello;
 mod mapdir;
 mod quine;

lib/wasi/wasitests/fs_sandbox_test.out

@@ -0,0 +1 @@
+Reading the parent directory was okay? false

lib/wasi/wasitests/fs_sandbox_test.rs

@@ -0,0 +1,10 @@
+fn main() {
+    #[cfg(target = "wasi")]
+    let result = std::fs::read_dir("..");
+    #[cfg(not(target = "wasi"))]
+    let result: Result<(), String> = Err("placeholder".to_string());
+    println!(
+        "Reading the parent directory was okay? {:?}",
+        result.is_ok()
+    );
+}