bytecodealliance / rustix

Safe Rust bindings to POSIX-ish APIs
Other
1.4k stars 139 forks source link

Fix soundness issues in `dup2_stdout` et al #1080

Closed pekkarr closed 2 weeks ago

pekkarr commented 3 weeks ago

The dup2_std* functions had a few soundness issues that allowed safe code to close the stdio file descriptors.

  1. AsFd::as_fd() was called twice, possibly returning a different BorrowedFd each time. This would allow safe code to bypass the fd.as_fd().as_raw_fd() != c::STDOUT_FILENO check.
  2. AsFd::as_fd() was called between let mut target = unsafe { take_stdout() }; and forget(target);. If as_fd() panics, then target would be automatically dropped without the call to forget. This violates exception safety.
  3. If backend::io::syscalls::dup2 returns an error, this was immediately returned from the function using the ? operator, so forget(target); wasn't called resulting in target getting dropped closing the stdio file descriptor.

Fix these issues by only calling AsFd::as_fd() once and by using ManuallyDrop instead of forget.

sunfishcode commented 2 weeks ago

Great catches, thanks!