Merge #463

463: Prevent parent directory from being opened without being preopened wasi r=MarkMcCaskey a=MarkMcCaskey

resolves  #462

Because the logic of opening a directory and traversing the tree are separate, we allowed one level of `..` to be opened beyond what was preopened

The diff on github isn't clear, but this adds an if and then puts the previous logic in an else block

Co-authored-by: Mark McCaskey <mark@wasmer.io>

CHANGELOG.md

@@ -6,6 +6,7 @@ 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

lib/wasi/src/syscalls/mod.rs

@@ -1463,10 +1463,28 @@ 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 {
+    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 {
+                return __WASI_EACCES;
+            }
+        } else {
             if let Some(child) = entries.get(file_name).cloned() {
                 let child_inode_val = &state.fs.inodes[child];
                 // early return based on flags
@@ -1488,6 +1506,7 @@ pub fn path_open(
                     .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
@@ -1555,6 +1574,7 @@ pub fn path_open(
 
                 new_fd
             }
+        }
     } else {
         // working_dir did not match on Kind::Dir
         return __WASI_ENOTDIR;

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()
+    );
+}