From 88671ed916f3374ae120f132015e7216cfe7687d Mon Sep 17 00:00:00 2001
From: "taylor.fish" <contact@taylor.fish>
Date: Mon, 18 Oct 2021 23:05:39 -0700
Subject: [PATCH] Fix unsoundness in `FdSet` methods

Ensure file descriptors are nonnegative and less than `FD_SETSIZE`.
(Fixes #1572.)
---
 CHANGELOG.md            | 13 +++++++++++++
 src/sys/select.rs       | 11 +++++++++++
 test/sys/test_select.rs | 28 ++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+)

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,
+    );
+}