Skip to content

Commit

Permalink
Fix UB in the SO_TYPE sockopt
Browse files Browse the repository at this point in the history
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 nix-rust#1819
  • Loading branch information
asomers authored and rtzoeller committed Dec 2, 2022
1 parent b9fa51f commit 3cda267
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 2 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
([#1815](https://github.com/nix-rust/nix/pull/1815))
- Fix `User::from_uid` and `User::from_name` crash on Android platform.
([#1824](https://github.com/nix-rust/nix/pull/1824))
- Fix UB with `sys::socket::sockopt::SockType` using `SOCK_PACKET`.
([#1821](https://github.com/nix-rust/nix/pull/1821))

### Removed

Expand Down
19 changes: 19 additions & 0 deletions src/sys/socket/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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 memoffset::offset_of;
use std::convert::TryFrom;
use std::{mem, ptr, slice};
use std::os::unix::io::RawFd;
#[cfg(all(target_os = "linux"))]
Expand Down Expand Up @@ -91,6 +92,24 @@ pub enum SockType {
/// guarantee ordering.
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.
Expand Down
8 changes: 6 additions & 2 deletions src/sys/socket/sockopt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::Result;
use crate::errno::Errno;
use crate::sys::time::TimeVal;
use libc::{self, c_int, c_void, socklen_t};
use std::convert::TryFrom;
use std::mem::{
self,
MaybeUninit
Expand Down Expand Up @@ -96,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)
}
}
}
}
Expand Down Expand Up @@ -413,7 +417,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)`.
Expand Down
27 changes: 27 additions & 0 deletions test/sys/test_sockopt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,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 beleived 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.
Expand Down

0 comments on commit 3cda267

Please sign in to comment.