bytecodealliance / wasmtime

A fast and secure runtime for WebAssembly
https://wasmtime.dev/
Apache License 2.0
15.1k stars 1.26k forks source link

Implicit "read" access when no rights specified in path_open on Unix #617

Open kubkon opened 4 years ago

kubkon commented 4 years ago

When discussing base and inheriting rights in path_open in #570, we've discovered that, on Unix, our implementation of path_open will implicitly open the derived descriptor with "read" access even when both rights_base and rights_inheriting are set to 0. @marmistrz and I have since been wondering whether this is the intended behaviour or not (for reference, here's the link to the offending bit: sys/unix/hostcalls_impl/fs.rs#L102). Note further that, if you specify __WASI_RIGHT_FD_WRITE, we don't implicitly inherit the read right. All in all then, the possible states for rights_base currently are:

This could have a potential unexpected behaviour in implementations wrapping the WASI syscalls such as std::fs::OpenOptions which assumes that __WASI_RIGHT_FD_READ is acquired only if the client has indeed specified that they want the path opened for reading which with the current behaviour will not always be the case. Here's the link to the relevant implementation bit in Rust: sys/wasi/fs.rs#L332.

kubkon commented 4 years ago

cc @sunfishcode @peterhuene @alexcrichton

marmistrz commented 4 years ago

I also created a test exposing this behavior: https://github.com/bytecodealliance/wasmtime/pull/618. We get read access even if we literally drop the right to call fd_read.

sunfishcode commented 4 years ago

The reason for opening with O_RDONLY when __WASI_RIGHT_FD_READ isn't set is because classic Unix doesn't have a way to open a file without either read or write access. But we may still do some operations on it, like fd_filestat_get, which require us to have some kind of open file descriptor.

fd_read should still disallow attempts to read if __WASI_RIGHT_FD_READ isn't set. Is that not happening?

Linux has O_PATH which is a way to open a file without read or write access, and in theory we could use that for these kinds of cases. That said, not all platforms have that, so it'd be good to figure out how to solve this without that too.

kubkon commented 4 years ago

@sunfishcode it seems that our Unix impl still allows fd_read with __WASI_RIGHT_FD_READ dropped (see @marmistrz’s test case in #618). Incidentally, we can observe the correct behaviour in our Windows impl, so I guess this is a bug on Unix :-)

marmistrz commented 4 years ago

The problem is that the derived fd will implicitly receive RIGHT_FD_READ, as shown in the new version of the testcase. The reason is that FdEntry::from will call determine_type_and_access_rights on the newly created descriptor, find out it's opened as O_RDONLY and conclude that the new file should have the right RIGHT_FD_READ.

alexcrichton commented 4 years ago

I don't personally have much to add here myself unfortunately, I'd be happy with whatever outcome!