Skip to content

Commit 0d74af3

Browse files
committed
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
1 parent 1038ee5 commit 0d74af3

File tree

4 files changed

+60
-6
lines changed

4 files changed

+60
-6
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
2020

2121
### Fixed
2222

23+
- Fix UB with `sys::socket::sockopt::SockType` using `SOCK_PACKET`.
24+
([#1821](https://github.com/nix-rust/nix/pull/1821))
2325
- Fix microsecond calculation for `TimeSpec`.
2426
([#1801](https://github.com/nix-rust/nix/pull/1801))
2527
- Fix `User::from_name` and `Group::from_name` panicking

src/sys/socket/mod.rs

+19-1
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use cfg_if::cfg_if;
55
use crate::{Result, errno::Errno};
66
use libc::{self, c_void, c_int, iovec, socklen_t, size_t,
77
CMSG_FIRSTHDR, CMSG_NXTHDR, CMSG_DATA, CMSG_LEN};
8-
use std::convert::TryInto;
8+
use std::convert::{TryFrom, TryInto};
99
use std::{mem, ptr, slice};
1010
use std::os::unix::io::RawFd;
1111
#[cfg(feature = "net")]
@@ -121,6 +121,24 @@ pub enum SockType {
121121
#[cfg(not(any(target_os = "haiku")))]
122122
Rdm = libc::SOCK_RDM,
123123
}
124+
// The TryFrom impl could've been derived using libc_enum!. But for
125+
// backwards-compatibility with Nix-0.25.0 we manually implement it, so as to
126+
// keep the old variant names.
127+
impl TryFrom<i32> for SockType {
128+
type Error = crate::Error;
129+
130+
fn try_from(x: i32) -> Result<Self> {
131+
match x {
132+
libc::SOCK_STREAM => Ok(Self::Stream),
133+
libc::SOCK_DGRAM => Ok(Self::Datagram),
134+
libc::SOCK_SEQPACKET => Ok(Self::SeqPacket),
135+
libc::SOCK_RAW => Ok(Self::Raw),
136+
#[cfg(not(any(target_os = "haiku")))]
137+
libc::SOCK_RDM => Ok(Self::Rdm),
138+
_ => Err(Errno::EINVAL)
139+
}
140+
}
141+
}
124142

125143
/// Constants used in [`socket`](fn.socket.html) and [`socketpair`](fn.socketpair.html)
126144
/// to specify the protocol to use.

src/sys/socket/sockopt.rs

+8-5
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ use crate::Result;
55
use crate::errno::Errno;
66
use crate::sys::time::TimeVal;
77
use libc::{self, c_int, c_void, socklen_t};
8-
use std::mem::{
9-
self,
10-
MaybeUninit
8+
use std::{
9+
convert::TryFrom,
10+
mem::{self, MaybeUninit}
1111
};
1212
use std::os::unix::io::RawFd;
1313
use std::ffi::{OsStr, OsString};
@@ -97,7 +97,10 @@ macro_rules! getsockopt_impl {
9797
getter.ffi_len());
9898
Errno::result(res)?;
9999

100-
Ok(getter.assume_init())
100+
match <$ty>::try_from(getter.assume_init()) {
101+
Err(_) => Err(Errno::EINVAL),
102+
Ok(r) => Ok(r)
103+
}
101104
}
102105
}
103106
}
@@ -449,7 +452,7 @@ sockopt_impl!(
449452
SndBufForce, SetOnly, libc::SOL_SOCKET, libc::SO_SNDBUFFORCE, usize);
450453
sockopt_impl!(
451454
/// Gets the socket type as an integer.
452-
SockType, GetOnly, libc::SOL_SOCKET, libc::SO_TYPE, super::SockType);
455+
SockType, GetOnly, libc::SOL_SOCKET, libc::SO_TYPE, super::SockType, GetStruct<i32>);
453456
sockopt_impl!(
454457
/// Returns a value indicating whether or not this socket has been marked to
455458
/// accept connections with `listen(2)`.

test/sys/test_sockopt.rs

+31
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,37 @@ fn test_so_tcp_maxseg() {
151151
close(ssock).unwrap();
152152
}
153153

154+
#[test]
155+
fn test_so_type() {
156+
let sockfd = socket(
157+
AddressFamily::Inet,
158+
SockType::Stream,
159+
SockFlag::empty(),
160+
None,
161+
)
162+
.unwrap();
163+
164+
assert_eq!(Ok(SockType::Stream), getsockopt(sockfd, sockopt::SockType));
165+
}
166+
167+
/// getsockopt(_, sockopt::SockType) should gracefully handle unknown socket
168+
/// types. Regression test for https://github.com/nix-rust/nix/issues/1819
169+
#[cfg(any(
170+
target_os = "android",
171+
target_os = "fuchsia",
172+
target_os = "linux",
173+
))]
174+
#[test]
175+
fn test_so_type_unknown() {
176+
use nix::errno::Errno;
177+
178+
require_capability!("test_so_type", CAP_NET_RAW);
179+
let sockfd = unsafe { libc::socket(libc::AF_PACKET, libc::SOCK_PACKET, 0) };
180+
assert!(sockfd >= 0, "Error opening socket: {}", nix::Error::last());
181+
182+
assert_eq!(Err(Errno::EINVAL), getsockopt(sockfd, sockopt::SockType));
183+
}
184+
154185
// The CI doesn't supported getsockopt and setsockopt on emulated processors.
155186
// It's believed that a QEMU issue, the tests run ok on a fully emulated system.
156187
// Current CI just run the binary with QEMU but the Kernel remains the same as the host.

0 commit comments

Comments
 (0)