Check if filetype of OS handle matches WASI filetype when creating

It seems prudent to check if the passed in `File` instance is of
type matching that of the requested WASI filetype. In other words,
we'd like to avoid situations where `OsFile` is created from a
pipe.

Author: Jakub Konka

crates/wasi-common/src/ctx.rs

@@ -1,7 +1,8 @@
 use crate::entry::{Entry, EntryHandle};
 use crate::fdpool::FdPool;
 use crate::handle::Handle;
-use crate::sys::osfile::{OsFile, OsFileExt};
+use crate::sys::osdir::OsDir;
+use crate::sys::osother::{OsOther, OsOtherExt};
 use crate::sys::stdio::{Stdio, StdioExt};
 use crate::virtfs::{VirtualDir, VirtualDirEntry};
 use crate::wasi::types;
@@ -120,9 +121,9 @@ pub struct WasiCtxBuilder {
 impl WasiCtxBuilder {
     /// Builder for a new `WasiCtx`.
     pub fn new() -> Self {
-        let stdin = Some(PendingEntry::Thunk(OsFile::from_null));
-        let stdout = Some(PendingEntry::Thunk(OsFile::from_null));
-        let stderr = Some(PendingEntry::Thunk(OsFile::from_null));
+        let stdin = Some(PendingEntry::Thunk(OsOther::from_null));
+        let stdout = Some(PendingEntry::Thunk(OsOther::from_null));
+        let stderr = Some(PendingEntry::Thunk(OsOther::from_null));
 
         Self {
             stdin,
@@ -253,7 +254,7 @@ impl WasiCtxBuilder {
     pub fn preopened_dir<P: AsRef<Path>>(&mut self, dir: File, guest_path: P) -> &mut Self {
         self.preopens.as_mut().unwrap().push((
             guest_path.as_ref().to_owned(),
-            <Box<dyn Handle>>::try_from(dir).expect("valid handle"),
+            Box::new(OsDir::try_from(dir).expect("valid OsDir handle")),
         ));
         self
     }
@@ -345,8 +346,8 @@ impl WasiCtxBuilder {
                         .ok_or(WasiCtxBuilderError::TooManyFilesOpen)?
                 }
                 PendingEntry::File(f) => {
-                    let handle = <Box<dyn Handle>>::try_from(f)?;
-                    let handle = EntryHandle::from(handle);
+                    let handle = OsOther::try_from(f)?;
+                    let handle = EntryHandle::new(handle);
                     let entry = Entry::new(handle);
                     entries
                         .insert(entry)

crates/wasi-common/src/entry.rs

@@ -8,6 +8,10 @@ use std::rc::Rc;
 pub(crate) struct EntryHandle(Rc<dyn Handle>);
 
 impl EntryHandle {
+    pub(crate) fn new<T: Handle + 'static>(handle: T) -> Self {
+        Self(Rc::new(handle))
+    }
+
     pub(crate) fn get(&self) -> Self {
         Self(Rc::clone(&self.0))
     }

crates/wasi-common/src/sys/osfile.rs

@@ -8,11 +8,6 @@ use std::fs::File;
 use std::io::{self, Read, Seek, SeekFrom, Write};
 use std::ops::Deref;
 
-pub(crate) trait OsFileExt {
-    /// Create `OsFile` as `dyn Handle` from null device.
-    fn from_null() -> io::Result<Box<dyn Handle>>;
-}
-
 #[derive(Debug)]
 pub(crate) struct OsFile {
     rights: Cell<HandleRights>,

crates/wasi-common/src/sys/osother.rs

@@ -10,6 +10,11 @@ use std::fs::File;
 use std::io::{self, Read, Write};
 use std::ops::Deref;
 
+pub(crate) trait OsOtherExt {
+    /// Create `OsOther` as `dyn Handle` from null device.
+    fn from_null() -> io::Result<Box<dyn Handle>>;
+}
+
 #[derive(Debug)]
 pub(crate) struct OsOther {
     file_type: Filetype,

crates/wasi-common/src/sys/unix/osdir.rs

@@ -12,6 +12,10 @@ impl TryFrom<File> for OsDir {
     type Error = io::Error;
 
     fn try_from(file: File) -> io::Result<Self> {
+        let ft = file.metadata()?.file_type();
+        if !ft.is_dir() {
+            return Err(io::Error::from_raw_os_error(libc::EINVAL));
+        }
         let rights = get_rights(&file)?;
         let handle = unsafe { OsHandle::from_raw_fd(file.into_raw_fd()) };
         Self::new(rights, handle)

crates/wasi-common/src/sys/unix/osfile.rs

@@ -1,16 +1,20 @@
 use super::oshandle::OsHandle;
-use crate::handle::{Handle, HandleRights};
-use crate::sys::osfile::{OsFile, OsFileExt};
+use crate::handle::HandleRights;
+use crate::sys::osfile::OsFile;
 use crate::wasi::{types, RightsExt};
 use std::convert::TryFrom;
-use std::fs::{File, OpenOptions};
+use std::fs::File;
 use std::io;
 use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd};
 
 impl TryFrom<File> for OsFile {
     type Error = io::Error;
 
     fn try_from(file: File) -> io::Result<Self> {
+        let ft = file.metadata()?.file_type();
+        if !ft.is_file() {
+            return Err(io::Error::from_raw_os_error(libc::EINVAL));
+        }
         let rights = get_rights(&file)?;
         let handle = unsafe { OsHandle::from_raw_fd(file.into_raw_fd()) };
         Ok(Self::new(rights, handle))
@@ -32,14 +36,3 @@ fn get_rights(file: &File) -> io::Result<HandleRights> {
     }
     Ok(rights)
 }
-
-impl OsFileExt for OsFile {
-    fn from_null() -> io::Result<Box<dyn Handle>> {
-        let file = OpenOptions::new()
-            .read(true)
-            .write(true)
-            .open("/dev/null")?;
-        let file = Self::try_from(file)?;
-        Ok(Box::new(file))
-    }
-}

crates/wasi-common/src/sys/unix/osother.rs

@@ -1,10 +1,10 @@
 use super::get_file_type;
 use super::oshandle::OsHandle;
-use crate::handle::HandleRights;
-use crate::sys::osother::OsOther;
+use crate::handle::{Handle, HandleRights};
+use crate::sys::osother::{OsOther, OsOtherExt};
 use crate::wasi::{types, RightsExt};
 use std::convert::TryFrom;
-use std::fs::File;
+use std::fs::{File, OpenOptions};
 use std::io;
 use std::os::unix::prelude::{AsRawFd, FromRawFd, IntoRawFd};
 
@@ -13,6 +13,9 @@ impl TryFrom<File> for OsOther {
 
     fn try_from(file: File) -> io::Result<Self> {
         let file_type = get_file_type(&file)?;
+        if file_type == types::Filetype::RegularFile || file_type == types::Filetype::Directory {
+            return Err(io::Error::from_raw_os_error(libc::EINVAL));
+        }
         let rights = get_rights(&file, &file_type)?;
         let handle = unsafe { OsHandle::from_raw_fd(file.into_raw_fd()) };
         Ok(Self::new(file_type, rights, handle))
@@ -57,3 +60,14 @@ fn get_rights(file: &File, file_type: &types::Filetype) -> io::Result<HandleRigh
     }
     Ok(rights)
 }
+
+impl OsOtherExt for OsOther {
+    fn from_null() -> io::Result<Box<dyn Handle>> {
+        let file = OpenOptions::new()
+            .read(true)
+            .write(true)
+            .open("/dev/null")?;
+        let file = Self::try_from(file)?;
+        Ok(Box::new(file))
+    }
+}

crates/wasi-common/src/sys/windows/osdir.rs

@@ -24,6 +24,10 @@ impl TryFrom<File> for OsDir {
     type Error = io::Error;
 
     fn try_from(file: File) -> io::Result<Self> {
+        let ft = file.metadata()?.file_type();
+        if !ft.is_dir() {
+            return Err(io::Error::from_raw_os_error(libc::EINVAL));
+        }
         let rights = get_rights(&file)?;
         let handle = unsafe { OsHandle::from_raw_handle(file.into_raw_handle()) };
         Self::new(rights, handle)

crates/wasi-common/src/sys/windows/osfile.rs

@@ -1,16 +1,20 @@
 use super::oshandle::OsHandle;
-use crate::handle::{Handle, HandleRights};
-use crate::sys::osfile::{OsFile, OsFileExt};
+use crate::handle::HandleRights;
+use crate::sys::osfile::OsFile;
 use crate::wasi::{types, RightsExt};
 use std::convert::TryFrom;
-use std::fs::{File, OpenOptions};
+use std::fs::File;
 use std::io;
 use std::os::windows::prelude::{AsRawHandle, FromRawHandle, IntoRawHandle};
 
 impl TryFrom<File> for OsFile {
     type Error = io::Error;
 
     fn try_from(file: File) -> io::Result<Self> {
+        let ft = file.metadata()?.file_type();
+        if !ft.is_file() {
+            return Err(io::Error::from_raw_os_error(libc::EINVAL));
+        }
         let rights = get_rights(&file)?;
         let handle = unsafe { OsHandle::from_raw_handle(file.into_raw_handle()) };
         Ok(Self::new(rights, handle))
@@ -32,11 +36,3 @@ fn get_rights(file: &File) -> io::Result<HandleRights> {
     }
     Ok(rights)
 }
-
-impl OsFileExt for OsFile {
-    fn from_null() -> io::Result<Box<dyn Handle>> {
-        let file = OpenOptions::new().read(true).write(true).open("NUL")?;
-        let file = Self::try_from(file)?;
-        Ok(Box::new(file))
-    }
-}

crates/wasi-common/src/sys/windows/osother.rs

@@ -1,10 +1,10 @@
 use super::get_file_type;
 use super::oshandle::OsHandle;
-use crate::handle::HandleRights;
-use crate::sys::osother::OsOther;
+use crate::handle::{Handle, HandleRights};
+use crate::sys::osother::{OsOther, OsOtherExt};
 use crate::wasi::{types, RightsExt};
 use std::convert::TryFrom;
-use std::fs::File;
+use std::fs::{File, OpenOptions};
 use std::io;
 use std::os::windows::prelude::{FromRawHandle, IntoRawHandle};
 
@@ -13,6 +13,9 @@ impl TryFrom<File> for OsOther {
 
     fn try_from(file: File) -> io::Result<Self> {
         let file_type = get_file_type(&file)?;
+        if file_type == types::Filetype::RegularFile || file_type == types::Filetype::Directory {
+            return Err(io::Error::from_raw_os_error(libc::EINVAL));
+        }
         let rights = get_rights(&file_type)?;
         let handle = unsafe { OsHandle::from_raw_handle(file.into_raw_handle()) };
         Ok(Self::new(file_type, rights, handle))
@@ -39,3 +42,11 @@ fn get_rights(file_type: &types::Filetype) -> io::Result<HandleRights> {
     let rights = HandleRights::new(base, inheriting);
     Ok(rights)
 }
+
+impl OsOtherExt for OsOther {
+    fn from_null() -> io::Result<Box<dyn Handle>> {
+        let file = OpenOptions::new().read(true).write(true).open("NUL")?;
+        let file = Self::try_from(file)?;
+        Ok(Box::new(file))
+    }
+}