fix(net): cap child-controlled sockaddr length before reading it#129
Conversation
|
Good catch! Instead of silent cap, is returning Errno(EINVAL) better? |
|
Agreed — EINVAL is the better call (it's what the kernel returns for Factored the six sockaddr reads through one Updated the regression test accordingly: the 4 GiB |
…ing 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 multikernel#128 (net parse/decide/execute refactor): the helper lives in network/mod.rs and the six reads are in the connect/send/unix submodules.
a60730c to
1750fd5
Compare
Problem
A sandboxed child can force a multi-GiB allocation in the supervisor by passing an oversized sockaddr length.
connect/sendto/sendmsg/sendmmsgtake the address length straight from child-controlled args (addr_len=args[5],msg_namelen, bothu32up to0xFFFFFFFF). The seccomp-notify trap fires at syscall entry, before the kernel's ownaddrlen > sizeof(sockaddr_storage)→EINVALcheck, so the length reachedread_child_memuncapped andread_child_mem_vmdidvec![0u8; len].Repro (any active net interception):
The supervisor allocates ~4 GiB for a sockaddr that is at most 128 bytes. Under default overcommit this is a repeatable transient memory-amplification; under
RLIMIT_AS/overcommit_memory=2/ low RAM the Rust allocation fails andhandle_alloc_erroraborts the whole supervisor — a fail-open crash of the security monitor that takes down every sandbox it runs.Every other child-controlled read is already bounded (
MAX_SEND_BUF64 MiB,MAX_CONTROL_BUF16 KiB, iovlen 1024, vlen 256). The sockaddr length was the one uncapped read.Fix
Clamp each of the six sockaddr reads to
MAX_SOCKADDR_LEN=size_of::<sockaddr_storage>()(128). The gate only needs the family and address bytes, and the actual send is either re-run by the kernel (Continue) or performed on-behalf with our own correctly-sized sockaddr, so no legitimate call (a real sockaddr is ≤ 128 bytes) is affected.Test
cargo buildclean; fullsandlock-corelib suite green (444), including the network tests.Notes
maindirectly since the bug is live there. If refactor(net): restructure the network module into parse/decide/execute phases #128 lands first, the same clamp applies to the splitnetwork/modules (trivial rebase).SO_DOMAIN-gated change acrosssendmsg/sendmmsg/sendtoand is kept out of this focused security fix.