Fix rights checks across the codebase.

crates/test-programs/wasi-tests/src/bin/fd_advise.rs

@@ -9,7 +9,11 @@ unsafe fn test_fd_advise(dir_fd: wasi::Fd) {
         0,
         "file",
         wasi::OFLAGS_CREAT,
-        wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
+        wasi::RIGHTS_FD_READ
+            | wasi::RIGHTS_FD_WRITE
+            | wasi::RIGHTS_FD_ADVISE
+            | wasi::RIGHTS_FD_FILESTAT_GET
+            | wasi::RIGHTS_FD_ALLOCATE,
         0,
         0,
     )

crates/test-programs/wasi-tests/src/bin/fd_filestat_set.rs

@@ -9,7 +9,11 @@ unsafe fn test_fd_filestat_set(dir_fd: wasi::Fd) {
         0,
         "file",
         wasi::OFLAGS_CREAT,
-        wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
+        wasi::RIGHTS_FD_READ
+            | wasi::RIGHTS_FD_WRITE
+            | wasi::RIGHTS_FD_FILESTAT_GET
+            | wasi::RIGHTS_FD_FILESTAT_SET_SIZE
+            | wasi::RIGHTS_FD_FILESTAT_SET_TIMES,
         0,
         0,
     )

crates/test-programs/wasi-tests/src/bin/fd_readdir.rs

@@ -90,7 +90,10 @@ unsafe fn test_fd_readdir(dir_fd: wasi::Fd) {
         0,
         "file",
         wasi::OFLAGS_CREAT,
-        wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
+        wasi::RIGHTS_FD_READ
+            | wasi::RIGHTS_FD_WRITE
+            | wasi::RIGHTS_FD_READDIR
+            | wasi::RIGHTS_FD_FILESTAT_GET,
         0,
         0,
     )

crates/test-programs/wasi-tests/src/bin/file_allocate.rs

@@ -9,7 +9,10 @@ unsafe fn test_file_allocate(dir_fd: wasi::Fd) {
         0,
         "file",
         wasi::OFLAGS_CREAT,
-        wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
+        wasi::RIGHTS_FD_READ
+            | wasi::RIGHTS_FD_WRITE
+            | wasi::RIGHTS_FD_ALLOCATE
+            | wasi::RIGHTS_FD_FILESTAT_GET,
         0,
         0,
     )

crates/test-programs/wasi-tests/src/bin/file_seek_tell.rs

@@ -9,7 +9,7 @@ unsafe fn test_file_seek_tell(dir_fd: wasi::Fd) {
         0,
         "file",
         wasi::OFLAGS_CREAT,
-        wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
+        wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE | wasi::RIGHTS_FD_SEEK | wasi::RIGHTS_FD_TELL,
         0,
         0,
     )

crates/test-programs/wasi-tests/src/bin/path_link.rs

@@ -2,8 +2,15 @@ use more_asserts::assert_gt;
 use std::{env, process};
 use wasi_tests::{create_file, open_scratch_directory};
 
+const TEST_RIGHTS: wasi::Rights = wasi::RIGHTS_FD_READ
+    | wasi::RIGHTS_PATH_LINK_SOURCE
+    | wasi::RIGHTS_PATH_LINK_TARGET
+    | wasi::RIGHTS_FD_FILESTAT_GET
+    | wasi::RIGHTS_PATH_OPEN
+    | wasi::RIGHTS_PATH_UNLINK_FILE;
+
 unsafe fn create_or_open(dir_fd: wasi::Fd, name: &str, flags: wasi::Oflags) -> wasi::Fd {
-    let file_fd = wasi::path_open(dir_fd, 0, name, flags, 0, 0, 0)
+    let file_fd = wasi::path_open(dir_fd, 0, name, flags, TEST_RIGHTS, TEST_RIGHTS, 0)
         .unwrap_or_else(|_| panic!("opening '{}'", name));
     assert_gt!(
         file_fd,
@@ -14,7 +21,7 @@ unsafe fn create_or_open(dir_fd: wasi::Fd, name: &str, flags: wasi::Oflags) -> w
 }
 
 unsafe fn open_link(dir_fd: wasi::Fd, name: &str) -> wasi::Fd {
-    let file_fd = wasi::path_open(dir_fd, 0, name, 0, 0, 0, 0)
+    let file_fd = wasi::path_open(dir_fd, 0, name, 0, TEST_RIGHTS, TEST_RIGHTS, 0)
         .unwrap_or_else(|_| panic!("opening a link '{}'", name));
     assert_gt!(
         file_fd,

crates/test-programs/wasi-tests/src/bin/path_open_read_without_rights.rs

@@ -0,0 +1,74 @@
+use std::{env, process};
+use wasi_tests::open_scratch_directory;
+use wasi_tests::{drop_rights, fd_get_rights, create_file};
+
+const TEST_FILENAME: &'static str = "file";
+
+unsafe fn try_read_file(dir_fd: wasi::Fd) {
+    let fd = wasi::path_open(dir_fd, 0, TEST_FILENAME, 0, 0, 0, 0).expect("opening the file");
+
+    // Check that we don't have the right to exeucute fd_read
+    let (rbase, rinher) = fd_get_rights(fd);
+    assert_eq!(
+        rbase & wasi::RIGHTS_FD_READ,
+        0,
+        "should not have base RIGHTS_FD_READ"
+    );
+    assert_eq!(
+        rinher & wasi::RIGHTS_FD_READ,
+        0,
+        "should not have inheriting RIGHTS_FD_READ"
+    );
+
+    let contents = &mut [0u8; 1];
+    let iovec = wasi::Iovec {
+        buf: contents.as_mut_ptr() as *mut _,
+        buf_len: contents.len(),
+    };
+    // Since we no longer have the right to fd_read, trying to read a file
+    // should be an error.
+    let err = wasi::fd_read(fd, &[iovec]).expect_err("reading bytes from file should fail");
+    assert_eq!(err, wasi::ERRNO_NOTCAPABLE, "the errno should be ENOTCAPABLE");
+}
+
+unsafe fn test_read_rights(dir_fd: wasi::Fd) {
+    create_file(dir_fd, TEST_FILENAME);
+    drop_rights(dir_fd, wasi::RIGHTS_FD_READ, wasi::RIGHTS_FD_READ);
+
+    let (rbase, rinher) = fd_get_rights(dir_fd);
+    assert_eq!(
+        rbase & wasi::RIGHTS_FD_READ,
+        0,
+        "dir should not have base RIGHTS_FD_READ"
+    );
+    assert_eq!(
+        rinher & wasi::RIGHTS_FD_READ,
+        0,
+        "dir should not have inheriting RIGHTS_FD_READ"
+    );
+
+    try_read_file(dir_fd);
+}
+
+fn main() {
+    let mut args = env::args();
+    let prog = args.next().unwrap();
+    let arg = if let Some(arg) = args.next() {
+        arg
+    } else {
+        eprintln!("usage: {} <scratch directory>", prog);
+        process::exit(1);
+    };
+
+    // Open scratch directory
+    let dir_fd = match open_scratch_directory(&arg) {
+        Ok(dir_fd) => dir_fd,
+        Err(err) => {
+            eprintln!("{}", err);
+            process::exit(1)
+        }
+    };
+
+    // Run the tests.
+    unsafe { test_read_rights(dir_fd) }
+}

crates/test-programs/wasi-tests/src/bin/poll_oneoff.rs

@@ -197,7 +197,7 @@ unsafe fn test_fd_readwrite_valid_fd(dir_fd: wasi::Fd) {
         0,
         "file",
         wasi::OFLAGS_CREAT,
-        wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE,
+        wasi::RIGHTS_FD_READ | wasi::RIGHTS_FD_WRITE | wasi::RIGHTS_POLL_FD_READWRITE,
         0,
         0,
     )

crates/test-programs/wasi-tests/src/lib.rs

@@ -24,7 +24,8 @@ pub fn open_scratch_directory(path: &str) -> Result<wasi::Fd, String> {
             }
             dst.set_len(stat.u.dir.pr_name_len);
             if dst == path.as_bytes() {
-                return Ok(wasi::path_open(i, 0, ".", wasi::OFLAGS_DIRECTORY, 0, 0, 0)
+                let (base, inherit) = fd_get_rights(i);
+                return Ok(wasi::path_open(i, 0, ".", wasi::OFLAGS_DIRECTORY, base, inherit, 0)
                     .expect("failed to open dir"));
             }
         }
@@ -43,3 +44,24 @@ pub unsafe fn create_file(dir_fd: wasi::Fd, filename: &str) {
     );
     wasi::fd_close(file_fd).expect("closing a file");
 }
+
+// Returns: (rights_base, rights_inheriting)
+pub unsafe fn fd_get_rights(fd: wasi::Fd) -> (wasi::Rights, wasi::Rights) {
+    let fdstat = wasi::fd_fdstat_get(fd).expect("fd_fdstat_get failed");
+    (fdstat.fs_rights_base, fdstat.fs_rights_inheriting)
+}
+
+pub unsafe fn drop_rights(
+    fd: wasi::Fd,
+    drop_base: wasi::Rights,
+    drop_inheriting: wasi::Rights,
+) {
+    let (current_base, current_inheriting) = fd_get_rights(fd);
+
+    let new_base = current_base & !drop_base;
+    let new_inheriting = current_inheriting & !drop_inheriting;
+
+        wasi::fd_fdstat_set_rights(fd, new_base, new_inheriting).expect(
+        "dropping fd rights",
+    );
+}

crates/wasi-common/src/fdentry.rs

@@ -155,8 +155,21 @@ impl FdEntry {
         rights_base: wasi::__wasi_rights_t,
         rights_inheriting: wasi::__wasi_rights_t,
     ) -> Result<()> {
-        if !self.rights_base & rights_base != 0 || !self.rights_inheriting & rights_inheriting != 0
-        {
+        let missing_base = !self.rights_base & rights_base;
+        let missing_inheriting = !self.rights_inheriting & rights_inheriting;
+        if missing_base != 0 || missing_inheriting != 0 {
+            log::trace!(
+                "     | validate_rights failed: required: \
+                 rights_base = {:#x}, rights_inheriting = {:#x}; \
+                 actual: rights_base = {:#x}, rights_inheriting = {:#x}; \
+                 missing_base = {:#x}, missing_inheriting = {:#x}",
+                rights_base,
+                rights_inheriting,
+                self.rights_base,
+                self.rights_inheriting,
+                missing_base,
+                missing_inheriting
+            );
             Err(Error::ENOTCAPABLE)
         } else {
             Ok(())

crates/wasi-common/src/hostcalls_impl/fs.rs

@@ -5,7 +5,6 @@ use crate::fdentry::{Descriptor, FdEntry};
 use crate::helpers::*;
 use crate::memory::*;
 use crate::sandboxed_tty_writer::SandboxedTTYWriter;
-use crate::sys::fdentry_impl::determine_type_rights;
 use crate::sys::hostcalls_impl::fs_helpers::path_open_rights;
 use crate::sys::{host_impl, hostcalls_impl};
 use crate::{helpers, host, wasi, wasi32, Error, Result};
@@ -308,7 +307,7 @@ pub(crate) unsafe fn fd_fdstat_set_flags(
 
     let fd = wasi_ctx
         .get_fd_entry(fd)?
-        .as_descriptor(0, 0)?
+        .as_descriptor(wasi::__WASI_RIGHTS_FD_FDSTAT_SET_FLAGS, 0)?
         .as_os_handle();
 
     hostcalls_impl::fd_fdstat_set_flags(&fd, fdflags)
@@ -574,6 +573,11 @@ pub(crate) unsafe fn path_open(
 
     let (needed_base, needed_inheriting) =
         path_open_rights(fs_rights_base, fs_rights_inheriting, oflags, fs_flags);
+    trace!(
+        "     | needed_base = {}, needed_inheriting = {}",
+        needed_base,
+        needed_inheriting
+    );
     let fe = wasi_ctx.get_fd_entry(dirfd)?;
     let resolved = path_get(
         fe,
@@ -593,13 +597,20 @@ pub(crate) unsafe fn path_open(
             | wasi::__WASI_RIGHTS_FD_FILESTAT_SET_SIZE)
         != 0;
 
+    trace!(
+        "     | calling path_open impl: read={}, write={}",
+        read,
+        write
+    );
     let fd = hostcalls_impl::path_open(resolved, read, write, oflags, fs_flags)?;
 
-    // Determine the type of the new file descriptor and which rights contradict with this type
-    let (_ty, max_base, max_inheriting) = determine_type_rights(&fd)?;
     let mut fe = FdEntry::from(fd)?;
-    fe.rights_base &= max_base;
-    fe.rights_inheriting &= max_inheriting;
+    // We need to manually deny the rights which are not explicitly requested.
+    // This should not be needed, but currently determine_type_and_access_rights,
+    // which is used by FdEntry::from, may grant extra rights while inferring it
+    // from the open mode.
+    fe.rights_base &= fs_rights_base;
+    fe.rights_inheriting &= fs_rights_inheriting;
     let guest_fd = wasi_ctx.insert_fd_entry(fe)?;
 
     trace!("     | *fd={:?}", guest_fd);
@@ -709,7 +720,10 @@ pub(crate) unsafe fn fd_filestat_get(
         filestat_ptr
     );
 
-    let fd = wasi_ctx.get_fd_entry(fd)?.as_descriptor(0, 0)?.as_file()?;
+    let fd = wasi_ctx
+        .get_fd_entry(fd)?
+        .as_descriptor(wasi::__WASI_RIGHTS_FD_FILESTAT_GET, 0)?
+        .as_file()?;
     let host_filestat = hostcalls_impl::fd_filestat_get(fd)?;
 
     trace!("     | *filestat_ptr={:?}", host_filestat);

crates/wasi-common/src/hostcalls_impl/misc.rs

@@ -242,9 +242,9 @@ pub(crate) fn poll_oneoff(
             {
                 let wasi_fd = unsafe { subscription.u.fd_readwrite.file_descriptor };
                 let rights = if r#type == wasi::__WASI_EVENTTYPE_FD_READ {
-                    wasi::__WASI_RIGHTS_FD_READ
+                    wasi::__WASI_RIGHTS_FD_READ | wasi::__WASI_RIGHTS_POLL_FD_READWRITE
                 } else {
-                    wasi::__WASI_RIGHTS_FD_WRITE
+                    wasi::__WASI_RIGHTS_FD_WRITE | wasi::__WASI_RIGHTS_POLL_FD_READWRITE
                 };
 
                 match unsafe {