Skip to content

Commit b351fe4

Browse files
dzerikcongwang-mk
authored andcommitted
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.
1 parent 1cd6ba6 commit b351fe4

5 files changed

Lines changed: 96 additions & 12 deletions

File tree

crates/sandlock-core/src/network/connect.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::os::unix::io::{AsRawFd, RawFd};
88
use std::sync::Arc;
99

1010
use crate::seccomp::ctx::SupervisorCtx;
11-
use crate::seccomp::notif::{read_child_mem, NotifAction};
11+
use crate::seccomp::notif::NotifAction;
1212
use crate::sys::structs::{SeccompNotif, ECONNREFUSED};
1313

1414
use super::materialize::{
@@ -42,9 +42,9 @@ pub(super) async fn connect_on_behalf(
4242

4343
// 1. Copy sockaddr from child memory
4444
let addr_bytes =
45-
match read_child_mem(notif_fd, notif.id, notif.pid, addr_ptr, addr_len as usize) {
45+
match super::read_sockaddr(notif_fd, notif.id, notif.pid, addr_ptr, addr_len as usize) {
4646
Ok(b) => b,
47-
Err(_) => return NotifAction::Errno(libc::EIO),
47+
Err(e) => return NotifAction::Errno(e),
4848
};
4949

5050
// 2. Check destination against the per-protocol endpoint allowlist.

crates/sandlock-core/src/network/mod.rs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use std::os::unix::io::RawFd;
2525
use std::sync::Arc;
2626

2727
use crate::seccomp::ctx::SupervisorCtx;
28-
use crate::seccomp::notif::NotifAction;
28+
use crate::seccomp::notif::{read_child_mem, NotifAction};
2929
use crate::sys::structs::SeccompNotif;
3030

3131
mod connect;
@@ -48,6 +48,37 @@ pub use rules::{
4848
use connect::connect_on_behalf;
4949
use send::{sendmmsg_on_behalf, sendmsg_on_behalf, sendto_on_behalf};
5050

51+
/// Largest sockaddr length we copy from the child when gating a `connect`/
52+
/// `sendto`/`sendmsg` destination. The seccomp-notify trap fires at syscall
53+
/// entry, *before* the kernel's own `addrlen > sizeof(sockaddr_storage)`
54+
/// (`EINVAL`) check, so a child can pass `addr_len`/`msg_namelen` up to
55+
/// `u32::MAX`. Reading that verbatim into `vec![0u8; len]` would let the child
56+
/// force a multi-GiB supervisor allocation (OOM / alloc-abort of the monitor)
57+
/// for an address that is at most this many bytes. Every legitimate sockaddr
58+
/// fits in `sizeof(sockaddr_storage)`, so a larger length is rejected before the
59+
/// read (see [`read_sockaddr`]).
60+
const MAX_SOCKADDR_LEN: usize = std::mem::size_of::<libc::sockaddr_storage>();
61+
62+
/// Copy a sockaddr from child memory, rejecting an oversized length *before* the
63+
/// allocation. `len` is child-controlled (`addr_len` / `msg_namelen`, a `u32`),
64+
/// and the trap fires before the kernel's `addrlen > sizeof(sockaddr_storage)`
65+
/// check, so an uncapped read would let the child force a multi-GiB supervisor
66+
/// allocation. A length larger than a `sockaddr_storage` cannot address a valid
67+
/// sockaddr, so it fails closed with `EINVAL` (matching what the kernel would
68+
/// return) rather than being silently truncated; a read fault maps to `EIO`.
69+
pub(super) fn read_sockaddr(
70+
notif_fd: RawFd,
71+
id: u64,
72+
pid: u32,
73+
ptr: u64,
74+
len: usize,
75+
) -> Result<Vec<u8>, i32> {
76+
if len > MAX_SOCKADDR_LEN {
77+
return Err(libc::EINVAL);
78+
}
79+
read_child_mem(notif_fd, id, pid, ptr, len).map_err(|_| libc::EIO)
80+
}
81+
5182
// ============================================================
5283
// query_socket_protocol — derive the rule Protocol from a fd via getsockopt
5384
// ============================================================

crates/sandlock-core/src/network/send.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ pub(super) async fn sendto_on_behalf(
6161

6262
// 1. Copy sockaddr from child memory (small: 16-28 bytes)
6363
let addr_bytes =
64-
match read_child_mem(notif_fd, notif.id, notif.pid, addr_ptr, addr_len as usize) {
64+
match super::read_sockaddr(notif_fd, notif.id, notif.pid, addr_ptr, addr_len as usize) {
6565
Ok(b) => b,
66-
Err(_) => return NotifAction::Errno(libc::EIO),
66+
Err(e) => return NotifAction::Errno(e),
6767
};
6868

6969
// 2. Check (ip, port) against the per-protocol endpoint allowlist.
@@ -241,9 +241,9 @@ fn prescan_msghdr(
241241
if hdr.connected() {
242242
return PrescanResult::ContinueWholeCall;
243243
}
244-
let addr_bytes = match read_child_mem(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize) {
244+
let addr_bytes = match super::read_sockaddr(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize) {
245245
Ok(b) => b,
246-
Err(_) => return PrescanResult::Errno(libc::EIO),
246+
Err(e) => return PrescanResult::Errno(e),
247247
};
248248
if parse_ip_from_sockaddr(&addr_bytes).is_none() {
249249
return PrescanResult::ContinueWholeCall;
@@ -291,9 +291,9 @@ async fn send_msghdr_on_behalf(
291291
let addr_bytes = if connected {
292292
Vec::new()
293293
} else {
294-
match read_child_mem(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize) {
294+
match super::read_sockaddr(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize) {
295295
Ok(b) => b,
296-
Err(_) => return Err(libc::EIO),
296+
Err(e) => return Err(e),
297297
}
298298
};
299299
if !connected {

crates/sandlock-core/src/network/unix.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ pub(super) fn unix_sendmsg_gate(
175175
return None; // connected socket: no address to gate
176176
}
177177
let addr_bytes =
178-
read_child_mem(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize).ok()?;
178+
super::read_sockaddr(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize).ok()?;
179179
// None unless this is a NAMED AF_UNIX target; IP/abstract fall through.
180180
let path = named_unix_socket_path(&addr_bytes)?;
181181

@@ -271,7 +271,7 @@ pub(super) fn mmsg_entry_named_unix_path(
271271
return None;
272272
}
273273
let addr_bytes =
274-
read_child_mem(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize).ok()?;
274+
super::read_sockaddr(notif_fd, notif.id, notif.pid, hdr.name_ptr, hdr.namelen as usize).ok()?;
275275
named_unix_socket_path(&addr_bytes)
276276
}
277277

crates/sandlock-core/tests/integration/test_network.rs

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,59 @@ async fn test_udp_rule_scopes_destination_by_host() {
6262
assert_eq!(blocked, "ERR:111", "sendto to disallowed host should ECONNREFUSED");
6363
}
6464

65+
/// Regression for the uncapped-sockaddr-length DoS: a child can pass
66+
/// `addr_len = 0xFFFFFFFF` to `sendto`. The seccomp-notify trap fires before the
67+
/// kernel's `addrlen > sizeof(sockaddr_storage) -> EINVAL` check, so without the
68+
/// `read_sockaddr` guard the supervisor did `vec![0u8; 0xFFFFFFFF]` (~4 GiB) and
69+
/// could OOM/abort, taking down every sandbox. The child sends with a bogus 4 GiB
70+
/// `addr_len` via raw `libc.sendto`. The supervisor must (a) survive — the whole
71+
/// run completes — and (b) reject the oversized length with `EINVAL` (22),
72+
/// matching the kernel, rather than reading it (or silently truncating). The
73+
/// destination gate never runs, so both the allowed and blocked host fail the
74+
/// same way.
75+
#[tokio::test]
76+
async fn test_sendto_huge_addrlen_rejected_with_einval() {
77+
let out = temp_file("huge-addrlen");
78+
79+
let policy = base_policy()
80+
.net_allow("udp://127.0.0.1:53")
81+
.build()
82+
.unwrap();
83+
84+
let script = format!(concat!(
85+
"import ctypes, socket, struct, os\n",
86+
"libc = ctypes.CDLL('libc.so.6', use_errno=True)\n",
87+
"libc.sendto.restype = ctypes.c_ssize_t\n",
88+
"def sai(ip, port):\n",
89+
" return struct.pack('<H', socket.AF_INET) + struct.pack('!H', port) + socket.inet_aton(ip) + b'\\x00'*8\n",
90+
"s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)\n",
91+
"buf = ctypes.create_string_buffer(b'x')\n",
92+
"def send(ip):\n",
93+
" a = ctypes.create_string_buffer(sai(ip, 53))\n",
94+
// addrlen = 0xFFFFFFFF: without the clamp this forces a ~4 GiB supervisor alloc.
95+
" ctypes.set_errno(0)\n",
96+
" r = libc.sendto(s.fileno(), buf, 1, 0, a, 0xFFFFFFFF)\n",
97+
" return 'OK' if r >= 0 else f'ERR:{{ctypes.get_errno()}}'\n",
98+
"allowed = send('127.0.0.1')\n",
99+
"blocked = send('1.1.1.1')\n",
100+
"open('{out}', 'w').write(allowed + '|' + blocked)\n",
101+
"s.close()\n",
102+
), out = out.display());
103+
104+
let result = policy.clone().with_name("test").run_interactive(&["python3", "-c", &script])
105+
.await.unwrap();
106+
// The run completing at all is the core assertion: an OOM-aborted supervisor
107+
// would kill the child instead of letting it finish.
108+
assert!(result.success(), "supervisor should survive a 4 GiB addr_len; exit={:?}", result.code());
109+
110+
let got = std::fs::read_to_string(&out).unwrap_or_default();
111+
let _ = std::fs::remove_file(&out);
112+
// EINVAL (22) for both: the oversized addr_len is rejected before the
113+
// destination gate, so allowed vs blocked is never reached.
114+
assert_eq!(got, "ERR:22|ERR:22",
115+
"a bogus 4 GiB addr_len must be rejected with EINVAL, matching the kernel");
116+
}
117+
65118
/// `udp://*:*` is the "any UDP destination" gate — it should not regress
66119
/// after Phase 2's per-protocol routing. Both sendtos succeed.
67120
#[tokio::test]

0 commit comments

Comments
 (0)