diff options
author | taylor.fish <contact@taylor.fish> | 2021-10-18 23:05:39 -0700 |
---|---|---|
committer | taylor.fish <contact@taylor.fish> | 2021-10-21 21:06:04 -0700 |
commit | dc80b4cd52f7195380f9848cd780138c8a7d21f9 (patch) | |
tree | b1c4bc150391dd35f8b8331ccfaab93b56b3d1f1 | |
parent | 103b29ffa7f895614ff0c2f9d26774701112f4af (diff) | |
download | nix-dc80b4cd52f7195380f9848cd780138c8a7d21f9.zip |
Fix unsoundness in `FdSet` methods
Ensure file descriptors are nonnegative and less than `FD_SETSIZE`.
(Fixes #1572.)
-rw-r--r-- | CHANGELOG.md | 13 | ||||
-rw-r--r-- | src/sys/select.rs | 11 | ||||
-rw-r--r-- | test/sys/test_select.rs | 28 |
3 files changed, 52 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index ccbb3562..d2db7d2d 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 5f3337f3..4d7576a5 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 37951086..db079456 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, + ); +} |