rust-secure-code / safety-dance

Auditing crates for unsafe code which can be safely replaced
Apache License 2.0
536 stars 10 forks source link

Audit Trillium #80

Open 8573 opened 2 years ago

8573 commented 2 years ago

Trillium "is a composable toolkit for building web applications". It is not popular (727 downloads/month), but it has very little unsafe in its own code: just two calls to std::net::TcpListener::from_raw_fd, with the file descriptor number taken from an environment variable.

Edited to add: Trillium is mostly under #[forbid(unsafe_code)].

Shnatsel commented 2 years ago

I believe OwnedFd can be used to avoid unsafe in this case, as detailed here. It's a very recent addition, but it is stable.

8573 commented 2 years ago

I note that the "Safety" documentation for from_raw_fd says

The fd passed in must be a valid and open file descriptor.

As far as I saw, Trillium does not check that the file descriptor numbers refer to valid, open file descriptors before calling from_raw_fd. Is that unsound?

Shnatsel commented 2 years ago

That's unsound, yes. But It's quite strange to read a file descriptor number of all the things from an environment variable; it would be quite difficult to specify what file you wish to operate on. This suggests that it may be a testing facility or something of the sort. In any case, this does warrant a closer look.

8573 commented 2 years ago

It's quite strange to read a file descriptor number of all the things from an environment variable

I think I found the explanation:

Trillium seeks to abide by a 12 factor approach to configuration, accepting configuration from the environment wherever possible. [...] In addition to accepting the HOST and PORT configuration from the environment, on cfg(unix) systems, trillium will also pick up a LISTEN_FD environment variable for use with catflap/systemfd.