From b5852bf5ff8215c3566dfa35796d61e06c67ff0e Mon Sep 17 00:00:00 2001
From: Pat Hickey <pat@moreproductive.org>
Date: Tue, 15 Dec 2020 16:44:03 -0800
Subject: [PATCH] open_dir can never create a dir

---
 crates/wasi-c2/src/dir.rs                     | 32 ++++++++-----------
 crates/wasi-c2/src/snapshots/preview_1.rs     | 16 +++++++---
 .../src/snapshots/wasi_snapshot_preview1.rs   |  2 +-
 3 files changed, 26 insertions(+), 24 deletions(-)

diff --git a/crates/wasi-c2/src/dir.rs b/crates/wasi-c2/src/dir.rs
index 8736f09fb6..9a3b9e6403 100644
--- a/crates/wasi-c2/src/dir.rs
+++ b/crates/wasi-c2/src/dir.rs
@@ -15,12 +15,7 @@ pub trait WasiDir {
         caps: FileCaps,
     ) -> Result<Box<dyn WasiFile>, Error>;
 
-    fn open_dir(
-        &self,
-        symlink_follow: bool,
-        path: &str,
-        create: bool,
-    ) -> Result<Box<dyn WasiDir>, Error>;
+    fn open_dir(&self, symlink_follow: bool, path: &str) -> Result<Box<dyn WasiDir>, Error>;
 
     fn readdir(
         &self,
@@ -150,9 +145,10 @@ impl WasiDir for cap_std::fs::Dir {
         oflags: OFlags,
         caps: FileCaps,
     ) -> Result<Box<dyn WasiFile>, Error> {
-        // XXX obey symlink_follow
-        // XXX how to handle fdflags like append? OFlags dont contain read|write?
+        use cap_fs_ext::{FollowSymlinks, OpenOptionsFollowExt};
+
         let mut opts = cap_std::fs::OpenOptions::new();
+
         if oflags.contains(&(OFlags::CREATE | OFlags::EXCLUSIVE)) {
             opts.create_new(true);
         } else if oflags.contains(&OFlags::CREATE) {
@@ -164,23 +160,23 @@ impl WasiDir for cap_std::fs::Dir {
         if caps.contains(&FileCaps::READ) {
             opts.read(true);
         }
-        if caps.contains(&FileCaps::WRITE) {
+        if caps.contains(&FileCaps::WRITE)
+            || caps.contains(&FileCaps::DATASYNC)
+            || caps.contains(&FileCaps::ALLOCATE)
+            || caps.contains(&FileCaps::FILESTAT_SET_SIZE)
+        {
             opts.write(true);
         }
+        if symlink_follow {
+            opts.follow(FollowSymlinks::Yes);
+        }
 
-        debug!("Dir::open_file({:?}, {:?})", path, opts);
         let f = self.open_with(Path::new(path), &opts)?;
-        debug!("succeeded");
         Ok(Box::new(f))
     }
 
-    fn open_dir(
-        &self,
-        symlink_follow: bool,
-        path: &str,
-        create: bool,
-    ) -> Result<Box<dyn WasiDir>, Error> {
-        // XXX obey symlink_follow, create
+    fn open_dir(&self, symlink_follow: bool, path: &str) -> Result<Box<dyn WasiDir>, Error> {
+        // XXX obey symlink_follow
         let d = self.open_dir(Path::new(path))?;
         Ok(Box::new(d))
     }
diff --git a/crates/wasi-c2/src/snapshots/preview_1.rs b/crates/wasi-c2/src/snapshots/preview_1.rs
index 062fe2a1f5..a6b1bdd816 100644
--- a/crates/wasi-c2/src/snapshots/preview_1.rs
+++ b/crates/wasi-c2/src/snapshots/preview_1.rs
@@ -168,6 +168,7 @@ impl<'a> wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
         let mut table = self.table();
         let fd = u32::from(fd);
 
+        // Fail fast: If not present in table, Badf
         if !table.contains_key(fd) {
             return Err(Error::Badf);
         }
@@ -183,7 +184,7 @@ impl<'a> wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
             drop(dir_entry);
             let _ = table.delete(fd);
         } else {
-            // XXX do we just table delete anyway?
+            // XXX do we just table delete other entry types anyway?
             return Err(Error::Badf);
         }
 
@@ -573,16 +574,21 @@ impl<'a> wasi_snapshot_preview1::WasiSnapshotPreview1 for WasiCtx {
         let mut table = self.table();
         let dir_entry: RefMut<DirEntry> = table.get(u32::from(dirfd))?;
         let dir = dir_entry.get_cap(DirCaps::OPEN)?;
+
         let symlink_follow = dirflags.contains(&types::Lookupflags::SYMLINK_FOLLOW);
+
         let oflags = OFlags::from(&oflags);
         let fdflags = FdFlags::from(&fdflags);
         let path = path.as_str()?;
         if oflags.contains(&OFlags::DIRECTORY) {
-            let create = oflags.contains(&OFlags::CREATE);
-            let child_dir = dir.open_dir(symlink_follow, path.deref(), create)?;
+            if oflags.contains(&OFlags::CREATE)
+                || oflags.contains(&OFlags::EXCLUSIVE)
+                || oflags.contains(&OFlags::TRUNCATE)
+            {
+                return Err(Error::Inval);
+            }
+            let child_dir = dir.open_dir(symlink_follow, path.deref())?;
 
-            // XXX go back and check these caps conversions - probably need to validate them
-            // against ???
             let base_caps = DirCaps::from(&fs_rights_base);
             let inheriting_caps = DirCaps::from(&fs_rights_inheriting);
             drop(dir);
diff --git a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs
index 3e0bd6d4fc..b46bf41ec9 100644
--- a/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs
+++ b/crates/wasi-common/src/snapshots/wasi_snapshot_preview1.rs
@@ -82,7 +82,7 @@ impl<'a> WasiSnapshotPreview1 for WasiCtx {
                 return Err(Error::Notsup);
             }
         }
-        self.remove_entry(fd)?;
+        let _ = self.remove_entry(fd)?;
         Ok(())
     }