From 6408622947b088982a26be40b6e35c04d896fad2 Mon Sep 17 00:00:00 2001
From: Alan Somers <asomers@gmail.com>
Date: Thu, 15 Sep 2022 14:58:56 -0600
Subject: [PATCH] Fix UB in the SO_TYPE sockopt

When reading a value into an enum from getsockopt, we must validate it.
Failing to do so can lead to UB for example with SOCK_PACKET on Linux.

Perform the validation in GetSockOpt::get.  Currently SockType is the
only type that requires validation.

Fixes #1819
---
 CHANGELOG.md              |  2 ++
 src/sys/socket/mod.rs     | 20 +++++++++++++++++++-
 src/sys/socket/sockopt.rs | 13 ++++++++-----
 test/sys/test_sockopt.rs  | 27 +++++++++++++++++++++++++++
 4 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 74bf755c09..884aa944a8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -20,6 +20,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 
 ### Fixed
 
+- Fix UB with `sys::socket::sockopt::SockType` using `SOCK_PACKET`.
+  ([#1821](https://github.com/nix-rust/nix/pull/1821))
 - Fix microsecond calculation for `TimeSpec`.
   ([#1801](https://github.com/nix-rust/nix/pull/1801))
 - Fix `User::from_name` and `Group::from_name` panicking
diff --git a/src/sys/socket/mod.rs b/src/sys/socket/mod.rs
index 6677420551..470ce7cd39 100644
--- a/src/sys/socket/mod.rs
+++ b/src/sys/socket/mod.rs
@@ -5,7 +5,7 @@ use cfg_if::cfg_if;
 use crate::{Result, errno::Errno};
 use libc::{self, c_void, c_int, iovec, socklen_t, size_t,
         CMSG_FIRSTHDR, CMSG_NXTHDR, CMSG_DATA, CMSG_LEN};
-use std::convert::TryInto;
+use std::convert::{TryFrom, TryInto};
 use std::{mem, ptr, slice};
 use std::os::unix::io::RawFd;
 #[cfg(feature = "net")]
@@ -121,6 +121,24 @@ pub enum SockType {
     #[cfg(not(any(target_os = "haiku")))]
     Rdm = libc::SOCK_RDM,
 }
+// The TryFrom impl could've been derived using libc_enum!.  But for
+// backwards-compatibility with Nix-0.25.0 we manually implement it, so as to
+// keep the old variant names.
+impl TryFrom<i32> for SockType {
+    type Error = crate::Error;
+
+    fn try_from(x: i32) -> Result<Self> {
+        match x {
+            libc::SOCK_STREAM => Ok(Self::Stream),
+            libc::SOCK_DGRAM => Ok(Self::Datagram),
+            libc::SOCK_SEQPACKET => Ok(Self::SeqPacket),
+            libc::SOCK_RAW => Ok(Self::Raw),
+            #[cfg(not(any(target_os = "haiku")))]
+            libc::SOCK_RDM => Ok(Self::Rdm),
+            _ => Err(Errno::EINVAL)
+        }
+    }
+}
 
 /// Constants used in [`socket`](fn.socket.html) and [`socketpair`](fn.socketpair.html)
 /// to specify the protocol to use.
diff --git a/src/sys/socket/sockopt.rs b/src/sys/socket/sockopt.rs
index b3828b316f..1bb4cecea0 100644
--- a/src/sys/socket/sockopt.rs
+++ b/src/sys/socket/sockopt.rs
@@ -5,9 +5,9 @@ use crate::Result;
 use crate::errno::Errno;
 use crate::sys::time::TimeVal;
 use libc::{self, c_int, c_void, socklen_t};
-use std::mem::{
-    self,
-    MaybeUninit
+use std::{
+    convert::TryFrom,
+    mem::{self, MaybeUninit}
 };
 use std::os::unix::io::RawFd;
 use std::ffi::{OsStr, OsString};
@@ -97,7 +97,10 @@ macro_rules! getsockopt_impl {
                                                getter.ffi_len());
                     Errno::result(res)?;
 
-                    Ok(getter.assume_init())
+                    match <$ty>::try_from(getter.assume_init()) {
+                        Err(_) => Err(Errno::EINVAL),
+                        Ok(r) => Ok(r)
+                    }
                 }
             }
         }
@@ -449,7 +452,7 @@ sockopt_impl!(
     SndBufForce, SetOnly, libc::SOL_SOCKET, libc::SO_SNDBUFFORCE, usize);
 sockopt_impl!(
     /// Gets the socket type as an integer.
-    SockType, GetOnly, libc::SOL_SOCKET, libc::SO_TYPE, super::SockType);
+    SockType, GetOnly, libc::SOL_SOCKET, libc::SO_TYPE, super::SockType, GetStruct<i32>);
 sockopt_impl!(
     /// Returns a value indicating whether or not this socket has been marked to
     /// accept connections with `listen(2)`.
diff --git a/test/sys/test_sockopt.rs b/test/sys/test_sockopt.rs
index 2ddbf77b83..1dbdf19b15 100644
--- a/test/sys/test_sockopt.rs
+++ b/test/sys/test_sockopt.rs
@@ -151,6 +151,33 @@ fn test_so_tcp_maxseg() {
     close(ssock).unwrap();
 }
 
+#[test]
+fn test_so_type() {
+    let sockfd = socket(
+        AddressFamily::Inet,
+        SockType::Stream,
+        SockFlag::empty(),
+        None,
+    )
+    .unwrap();
+
+    assert_eq!(Ok(SockType::Stream), getsockopt(sockfd, sockopt::SockType));
+}
+
+/// getsockopt(_, sockopt::SockType) should gracefully handle unknown socket
+/// types.  Regression test for https://github.com/nix-rust/nix/issues/1819
+#[cfg(any(target_os = "android", target_os = "linux",))]
+#[test]
+fn test_so_type_unknown() {
+    use nix::errno::Errno;
+
+    require_capability!("test_so_type", CAP_NET_RAW);
+    let sockfd = unsafe { libc::socket(libc::AF_PACKET, libc::SOCK_PACKET, 0) };
+    assert!(sockfd >= 0, "Error opening socket: {}", nix::Error::last());
+
+    assert_eq!(Err(Errno::EINVAL), getsockopt(sockfd, sockopt::SockType));
+}
+
 // The CI doesn't supported getsockopt and setsockopt on emulated processors.
 // It's believed that a QEMU issue, the tests run ok on a fully emulated system.
 // Current CI just run the binary with QEMU but the Kernel remains the same as the host.