From 1750fd585ff82516d87d35cc7ca0f96356066353 Mon Sep 17 00:00:00 2001 From: dzerik Date: Fri, 10 Jul 2026 15:42:59 +0300 Subject: [PATCH] fix(net): reject an oversized sockaddr length with EINVAL before reading it The seccomp-notify trap fires at syscall entry, before the kernel's own `addrlen > sizeof(sockaddr_storage)` -> EINVAL check, so a child can pass an `addr_len`/`msg_namelen` up to u32::MAX. Reading it verbatim into `vec![0u8; len]` let the child force a multi-GiB supervisor allocation (OOM / alloc-abort of the monitor). Add a `read_sockaddr` helper that rejects a length larger than a `sockaddr_storage` with EINVAL (matching the kernel) before the allocation, and route the six connect/sendto/sendmsg/sendmmsg sockaddr reads through it; a read fault still maps to EIO. Regression test drives a bogus 4 GiB addr_len and asserts EINVAL plus a surviving supervisor. Rebased onto main after #128 (net parse/decide/execute refactor): the helper lives in network/mod.rs and the six reads are in the connect/send/unix submodules. --- crates/sandlock-core/src/network/connect.rs | 6 +-- crates/sandlock-core/src/network/mod.rs | 33 +++++++++++- crates/sandlock-core/src/network/send.rs | 12 ++--- crates/sandlock-core/src/network/unix.rs | 4 +- .../tests/integration/test_network.rs | 53 +++++++++++++++++++ 5 files changed, 96 insertions(+), 12 deletions(-) diff --git a/crates/sandlock-core/src/network/connect.rs b/crates/sandlock-core/src/network/connect.rs index d9c4ca07..d7e40c32 100644 --- a/crates/sandlock-core/src/network/connect.rs +++ b/crates/sandlock-core/src/network/connect.rs @@ -8,7 +8,7 @@ use std::os::unix::io::{AsRawFd, RawFd}; use std::sync::Arc; use crate::seccomp::ctx::SupervisorCtx; -use crate::seccomp::notif::{read_child_mem, NotifAction}; +use crate::seccomp::notif::NotifAction; use crate::sys::structs::{SeccompNotif, ECONNREFUSED}; use super::materialize::{ @@ -42,9 +42,9 @@ pub(super) async fn connect_on_behalf( // 1. Copy sockaddr from child memory let addr_bytes = - match read_child_mem(notif_fd, notif.id, notif.pid, addr_ptr, addr_len as usize) { + match super::read_sockaddr(notif_fd, notif.id, notif.pid, addr_ptr, addr_len as usize) { Ok(b) => b, - Err(_) => return NotifAction::Errno(libc::EIO), + Err(e) => return NotifAction::Errno(e), }; // 2. Check destination against the per-protocol endpoint allowlist. diff --git a/crates/sandlock-core/src/network/mod.rs b/crates/sandlock-core/src/network/mod.rs index e1edfde2..528a5862 100644 --- a/crates/sandlock-core/src/network/mod.rs +++ b/crates/sandlock-core/src/network/mod.rs @@ -25,7 +25,7 @@ use std::os::unix::io::RawFd; use std::sync::Arc; use crate::seccomp::ctx::SupervisorCtx; -use crate::seccomp::notif::NotifAction; +use crate::seccomp::notif::{read_child_mem, NotifAction}; use crate::sys::structs::SeccompNotif; mod connect; @@ -48,6 +48,37 @@ pub use rules::{ use connect::connect_on_behalf; use send::{sendmmsg_on_behalf, sendmsg_on_behalf, sendto_on_behalf}; +/// Largest sockaddr length we copy from the child when gating a `connect`/ +/// `sendto`/`sendmsg` destination. The seccomp-notify trap fires at syscall +/// entry, *before* the kernel's own `addrlen > sizeof(sockaddr_storage)` +/// (`EINVAL`) check, so a child can pass `addr_len`/`msg_namelen` up to +/// `u32::MAX`. Reading that verbatim into `vec![0u8; len]` would let the child +/// force a multi-GiB supervisor allocation (OOM / alloc-abort of the monitor) +/// for an address that is at most this many bytes. Every legitimate sockaddr +/// fits in `sizeof(sockaddr_storage)`, so a larger length is rejected before the +/// read (see [`read_sockaddr`]). +const MAX_SOCKADDR_LEN: usize = std::mem::size_of::(); + +/// Copy a sockaddr from child memory, rejecting an oversized length *before* the +/// allocation. `len` is child-controlled (`addr_len` / `msg_namelen`, a `u32`), +/// and the trap fires before the kernel's `addrlen > sizeof(sockaddr_storage)` +/// check, so an uncapped read would let the child force a multi-GiB supervisor +/// allocation. A length larger than a `sockaddr_storage` cannot address a valid +/// sockaddr, so it fails closed with `EINVAL` (matching what the kernel would +/// return) rather than being silently truncated; a read fault maps to `EIO`. +pub(super) fn read_sockaddr( + notif_fd: RawFd, + id: u64, + pid: u32, + ptr: u64, + len: usize, +) -> Result, i32> { + if len > MAX_SOCKADDR_LEN { + return Err(libc::EINVAL); + } + read_child_mem(notif_fd, id, pid, ptr, len).map_err(|_| libc::EIO) +} + // ============================================================ // query_socket_protocol — derive the rule Protocol from a fd via getsockopt // ============================================================ diff --git a/crates/sandlock-core/src/network/send.rs b/crates/sandlock-core/src/network/send.rs index a02c1852..f717a7e5 100644 --- a/crates/sandlock-core/src/network/send.rs +++ b/crates/sandlock-core/src/network/send.rs @@ -61,9 +61,9 @@ pub(super) async fn sendto_on_behalf( // 1. Copy sockaddr from child memory (small: 16-28 bytes) let addr_bytes = - match read_child_mem(notif_fd, notif.id, notif.pid, addr_ptr, addr_len as usize) { + match super::read_sockaddr(notif_fd, notif.id, notif.pid, addr_ptr, addr_len as usize) { Ok(b) => b, - Err(_) => return NotifAction::Errno(libc::EIO), + Err(e) => return NotifAction::Errno(e), }; // 2. Check (ip, port) against the per-protocol endpoint allowlist. @@ -241,9 +241,9 @@ fn prescan_msghdr( if hdr.connected() { return PrescanResult::ContinueWholeCall; } - let addr_bytes = match read_child_mem(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize) { + let addr_bytes = match super::read_sockaddr(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize) { Ok(b) => b, - Err(_) => return PrescanResult::Errno(libc::EIO), + Err(e) => return PrescanResult::Errno(e), }; if parse_ip_from_sockaddr(&addr_bytes).is_none() { return PrescanResult::ContinueWholeCall; @@ -291,9 +291,9 @@ async fn send_msghdr_on_behalf( let addr_bytes = if connected { Vec::new() } else { - match read_child_mem(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize) { + match super::read_sockaddr(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize) { Ok(b) => b, - Err(_) => return Err(libc::EIO), + Err(e) => return Err(e), } }; if !connected { diff --git a/crates/sandlock-core/src/network/unix.rs b/crates/sandlock-core/src/network/unix.rs index bf4f4a70..2390a010 100644 --- a/crates/sandlock-core/src/network/unix.rs +++ b/crates/sandlock-core/src/network/unix.rs @@ -175,7 +175,7 @@ pub(super) fn unix_sendmsg_gate( return None; // connected socket: no address to gate } let addr_bytes = - read_child_mem(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize).ok()?; + super::read_sockaddr(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize).ok()?; // None unless this is a NAMED AF_UNIX target; IP/abstract fall through. let path = named_unix_socket_path(&addr_bytes)?; @@ -271,7 +271,7 @@ pub(super) fn mmsg_entry_named_unix_path( return None; } let addr_bytes = - read_child_mem(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize).ok()?; + super::read_sockaddr(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize).ok()?; named_unix_socket_path(&addr_bytes) } diff --git a/crates/sandlock-core/tests/integration/test_network.rs b/crates/sandlock-core/tests/integration/test_network.rs index 2e47e3d4..aa5613c1 100644 --- a/crates/sandlock-core/tests/integration/test_network.rs +++ b/crates/sandlock-core/tests/integration/test_network.rs @@ -62,6 +62,59 @@ async fn test_udp_rule_scopes_destination_by_host() { assert_eq!(blocked, "ERR:111", "sendto to disallowed host should ECONNREFUSED"); } +/// Regression for the uncapped-sockaddr-length DoS: a child can pass +/// `addr_len = 0xFFFFFFFF` to `sendto`. The seccomp-notify trap fires before the +/// kernel's `addrlen > sizeof(sockaddr_storage) -> EINVAL` check, so without the +/// `read_sockaddr` guard the supervisor did `vec![0u8; 0xFFFFFFFF]` (~4 GiB) and +/// could OOM/abort, taking down every sandbox. The child sends with a bogus 4 GiB +/// `addr_len` via raw `libc.sendto`. The supervisor must (a) survive — the whole +/// run completes — and (b) reject the oversized length with `EINVAL` (22), +/// matching the kernel, rather than reading it (or silently truncating). The +/// destination gate never runs, so both the allowed and blocked host fail the +/// same way. +#[tokio::test] +async fn test_sendto_huge_addrlen_rejected_with_einval() { + let out = temp_file("huge-addrlen"); + + let policy = base_policy() + .net_allow("udp://127.0.0.1:53") + .build() + .unwrap(); + + let script = format!(concat!( + "import ctypes, socket, struct, os\n", + "libc = ctypes.CDLL('libc.so.6', use_errno=True)\n", + "libc.sendto.restype = ctypes.c_ssize_t\n", + "def sai(ip, port):\n", + " return struct.pack('= 0 else f'ERR:{{ctypes.get_errno()}}'\n", + "allowed = send('127.0.0.1')\n", + "blocked = send('1.1.1.1')\n", + "open('{out}', 'w').write(allowed + '|' + blocked)\n", + "s.close()\n", + ), out = out.display()); + + let result = policy.clone().with_name("test").run_interactive(&["python3", "-c", &script]) + .await.unwrap(); + // The run completing at all is the core assertion: an OOM-aborted supervisor + // would kill the child instead of letting it finish. + assert!(result.success(), "supervisor should survive a 4 GiB addr_len; exit={:?}", result.code()); + + let got = std::fs::read_to_string(&out).unwrap_or_default(); + let _ = std::fs::remove_file(&out); + // EINVAL (22) for both: the oversized addr_len is rejected before the + // destination gate, so allowed vs blocked is never reached. + assert_eq!(got, "ERR:22|ERR:22", + "a bogus 4 GiB addr_len must be rejected with EINVAL, matching the kernel"); +} + /// `udp://*:*` is the "any UDP destination" gate — it should not regress /// after Phase 2's per-protocol routing. Both sendtos succeed. #[tokio::test]