diff --git a/CHANGELOG.md b/CHANGELOG.md index ccbb3562ec..d2db7d2d61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,19 @@ All notable changes to this project will be documented in this file. This project adheres to [Semantic Versioning](https://semver.org/). +## [Unreleased] - ReleaseDate + +### Added +### Changed +### Fixed + +- Fixed soundness issues in `FdSet::insert`, `FdSet::remove`, and + `FdSet::contains` involving file descriptors outside of the range + `0..FD_SETSIZE`. + (#[1575](https://github.com/nix-rust/nix/pull/1575)) + +### Removed + ## [0.23.0] - 2021-09-28 ### Added diff --git a/src/sys/select.rs b/src/sys/select.rs index 5f3337f352..4d7576a58a 100644 --- a/src/sys/select.rs +++ b/src/sys/select.rs @@ -1,4 +1,5 @@ //! Portably monitor a group of file descriptors for readiness. +use std::convert::TryFrom; use std::iter::FusedIterator; use std::mem; use std::ops::Range; @@ -17,6 +18,13 @@ pub use libc::FD_SETSIZE; #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub struct FdSet(libc::fd_set); +fn assert_fd_valid(fd: RawFd) { + assert!( + usize::try_from(fd).map_or(false, |fd| fd < FD_SETSIZE), + "fd must be in the range 0..FD_SETSIZE", + ); +} + impl FdSet { /// Create an empty `FdSet` pub fn new() -> FdSet { @@ -29,16 +37,19 @@ impl FdSet { /// Add a file descriptor to an `FdSet` pub fn insert(&mut self, fd: RawFd) { + assert_fd_valid(fd); unsafe { libc::FD_SET(fd, &mut self.0) }; } /// Remove a file descriptor from an `FdSet` pub fn remove(&mut self, fd: RawFd) { + assert_fd_valid(fd); unsafe { libc::FD_CLR(fd, &mut self.0) }; } /// Test an `FdSet` for the presence of a certain file descriptor. pub fn contains(&self, fd: RawFd) -> bool { + assert_fd_valid(fd); unsafe { libc::FD_ISSET(fd, &self.0) } } diff --git a/test/sys/test_select.rs b/test/sys/test_select.rs index 37951086c2..db07945617 100644 --- a/test/sys/test_select.rs +++ b/test/sys/test_select.rs @@ -52,3 +52,31 @@ pub fn test_pselect_nfds2() { assert!(fd_set.contains(r1)); assert!(!fd_set.contains(r2)); } + +macro_rules! generate_fdset_bad_fd_tests { + ($fd:expr, $($method:ident),* $(,)?) => { + $( + #[test] + #[should_panic] + fn $method() { + FdSet::new().$method($fd); + } + )* + } +} + +mod test_fdset_negative_fd { + use super::*; + generate_fdset_bad_fd_tests!(-1, insert, remove, contains); +} + +mod test_fdset_too_large_fd { + use super::*; + use std::convert::TryInto; + generate_fdset_bad_fd_tests!( + FD_SETSIZE.try_into().unwrap(), + insert, + remove, + contains, + ); +}