From c4549956bac1089cb7e53743c565e096bcbdd369 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 11 Jul 2026 15:18:19 +0000 Subject: [PATCH] fix(net): guard NULL ifa_addr in IPv4 iface scan + validate extra-interface bridge name (1.1.24) Two low-severity, compile-gated fixes from the second-pass audit: - lib/net.cpp getIfaceIp4Addresses dereferenced a->ifa_addr->sa_family without a NULL check while the IPv6 sibling guards it. getifaddrs(3) can return entries with NULL ifa_addr (interface with no address, e.g. a fresh epair), so the IPv4 path could crash (local DoS). The loop now mirrors the IPv6 one: skip NULL ifa_addr, non-AF_INET, name mismatch, and NULL ifa_netmask. - lib/run.cpp extra-interfaces loop passed ex.bridgeIface straight into RunNet::createBridgeEpair -> `ifconfig addm ` with no validation, while the primary bridge path runs it through BridgePure::validateBridgeName. A leading-'-' value is taken as an ifconfig(8) option (argument injection). The loop now validates the name the same way. validateBridgeName (rejects leading dash / shell metachars / .. / over-length) is already covered by bridge_pure_test; the net.cpp change is runtime-only. Bumps to 1.1.24; CHANGELOG + trust-model (en/uk). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01X6t6tzVypHye5bDGLxzmZK --- CHANGELOG.md | 27 +++++++++++++++++++++++++ cli/args.cpp | 4 ++-- docs/trust-model.md | 4 ++-- docs/trust-model.uk.md | 4 ++-- lib/net.cpp | 45 +++++++++++++++++++++++++----------------- lib/run.cpp | 9 +++++++++ 6 files changed, 69 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11dbd21..5918e90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,33 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). --- +## [1.1.24] — 2026-07-07 + +**Robustness: two low-severity fixes from the second-pass audit that +don't need on-hardware validation (compile-gated by the FreeBSD build).** + +- **Null-pointer dereference in `getIfaceIp4Addresses` (`lib/net.cpp`).** + The IPv4 interface loop dereferenced `a->ifa_addr->sa_family` without + a NULL check, while the IPv6 sibling (`getIfaceIp6Addresses`) already + guards `ifa_addr == nullptr`. `getifaddrs(3)` can return entries with + a NULL `ifa_addr` (an interface with no address — e.g. a freshly + created epair before configuration), so the IPv4 path could crash + (local DoS). The loop now mirrors the IPv6 one: it skips NULL + `ifa_addr`, non-AF_INET, name mismatches, and NULL `ifa_netmask`. + +- **Extra-interface bridge name not validated (`lib/run.cpp`).** The + primary bridge path runs `optionNet->bridgeIface` through + `BridgePure::validateBridgeName`, but the extra-interfaces loop passed + `ex.bridgeIface` straight into `RunNet::createBridgeEpair` → + `ifconfig addm `. A leading-`-` value is taken as an + `ifconfig(8)` option (argument injection; no shell involved). The loop + now validates the name the same way as the primary path. + +`BridgePure::validateBridgeName` (which rejects the leading dash, shell +metacharacters, `..`, and over-length names) is already covered by +`bridge_pure_test`; the `net.cpp` change is runtime-only (no pure unit +surface). + ## [1.1.23] — 2026-07-07 **Security: decide daemon auth locality from the accepting socket, not diff --git a/cli/args.cpp b/cli/args.cpp index 5d078ad..bc7bddf 100644 --- a/cli/args.cpp +++ b/cli/args.cpp @@ -753,7 +753,7 @@ Args parseArguments(int argc, char** argv, unsigned &processed) { args.noColor = true; break; } else if (strEq(argv[a], "--version")) { - std::cout << "crate 1.1.23" << std::endl; + std::cout << "crate 1.1.24" << std::endl; exit(0); } else if (auto argShort = isShort(argv[a])) { switch (argShort) { @@ -764,7 +764,7 @@ Args parseArguments(int argc, char** argv, unsigned &processed) { args.logProgress = true; break; case 'V': - std::cout << "crate 1.1.23" << std::endl; + std::cout << "crate 1.1.24" << std::endl; exit(0); default: err("unsupported short option '%s'", argv[a]); diff --git a/docs/trust-model.md b/docs/trust-model.md index 80a1bbe..e289e02 100644 --- a/docs/trust-model.md +++ b/docs/trust-model.md @@ -4,7 +4,7 @@ operators on one machine) and contributors extending the privileged surface. -**Applies to:** 1.1.23 (rootless model + per-tenant authz series 1.1.12 → +**Applies to:** 1.1.24 (rootless model + per-tenant authz series 1.1.12 → 1.1.17 covering every privops verb that carries an operator-controlled ownership signal). For the ≤ 0.9.x setuid model and the migration, see [`rootless-migration.md`](rootless-migration.md). @@ -62,7 +62,7 @@ surface; 1.0.0 removed the setuid bit (`Makefile`, comment at the operator and delegates privileged operations to crated(8)"*). The single-trust-domain property did **not** disappear — it relocated. -Reasoning about isolation on 1.1.23 means reasoning about who can reach +Reasoning about isolation on 1.1.24 means reasoning about who can reach **privops**, not who can run `crate(1)`. --- diff --git a/docs/trust-model.uk.md b/docs/trust-model.uk.md index 582beca..112b684 100644 --- a/docs/trust-model.uk.md +++ b/docs/trust-model.uk.md @@ -4,7 +4,7 @@ (кілька операторів на одній машині), і контрибʼютори, які розширюють привілейовану поверхню. -**Стосується:** 1.1.23 (rootless-модель + серія per-tenant authz 1.1.12 → +**Стосується:** 1.1.24 (rootless-модель + серія per-tenant authz 1.1.12 → 1.1.17 покриває кожен privops-верб з operator-controlled ownership- сигналом). Про ≤ 0.9.x setuid-модель і міграцію див. [`rootless-migration.md`](rootless-migration.md). @@ -62,7 +62,7 @@ privops-сокета, ніколи admin-токен і ніколи Unix-сок privileged operations to crated(8)»*). Властивість «єдиний домен довіри» **не зникла** — вона переїхала. -Міркувати про ізоляцію на 1.1.23 — це міркувати про те, хто має доступ +Міркувати про ізоляцію на 1.1.24 — це міркувати про те, хто має доступ до **privops**, а не хто може запустити `crate(1)`. --- diff --git a/lib/net.cpp b/lib/net.cpp index 5b8a639..2c4c2c6 100644 --- a/lib/net.cpp +++ b/lib/net.cpp @@ -64,24 +64,33 @@ std::vector getIfaceIp4Addresses(const std::string &ifaceName) { }); // filter only IPv4 addresses for the requested interface - for (struct ifaddrs *a = ifap; a; a = a->ifa_next) - if (a->ifa_addr->sa_family == AF_INET && ::strcmp(a->ifa_name, ifaceName.c_str()) == 0) { // IPv4 for the requested interface - res = ::getnameinfo(a->ifa_addr, - sizeof(struct sockaddr_in), - host, NI_MAXHOST, - nullptr, 0, NI_NUMERICHOST); - if (res != 0) - ERR2("get network interface address", "getnameinfo() failed: " << ::gai_strerror(res)); - - res = ::getnameinfo(a->ifa_netmask, - sizeof(struct sockaddr_in), - netmask, NI_MAXHOST, - nullptr, 0, NI_NUMERICHOST); - if (res != 0) - ERR2("get network interface address", "getnameinfo() failed: " << ::gai_strerror(res)); - - addrs.push_back({host, netmask, netFromHostAndNatmaskV4(host, netmask)}); - } + for (struct ifaddrs *a = ifap; a; a = a->ifa_next) { + // 1.1.24: ifa_addr can be NULL (an interface with no address — e.g. + // a freshly-created epair before configuration). The IPv6 sibling + // below already guards this; the IPv4 path used to deref it blindly + // (a->ifa_addr->sa_family) and crash. ifa_netmask can be NULL too; + // without it we can't derive the network, so skip the entry. + if (a->ifa_addr == nullptr) continue; + if (a->ifa_addr->sa_family != AF_INET) continue; + if (::strcmp(a->ifa_name, ifaceName.c_str()) != 0) continue; + if (a->ifa_netmask == nullptr) continue; + + res = ::getnameinfo(a->ifa_addr, + sizeof(struct sockaddr_in), + host, NI_MAXHOST, + nullptr, 0, NI_NUMERICHOST); + if (res != 0) + ERR2("get network interface address", "getnameinfo() failed: " << ::gai_strerror(res)); + + res = ::getnameinfo(a->ifa_netmask, + sizeof(struct sockaddr_in), + netmask, NI_MAXHOST, + nullptr, 0, NI_NUMERICHOST); + if (res != 0) + ERR2("get network interface address", "getnameinfo() failed: " << ::gai_strerror(res)); + + addrs.push_back({host, netmask, netFromHostAndNatmaskV4(host, netmask)}); + } return addrs; } diff --git a/lib/run.cpp b/lib/run.cpp index 22640ec..14b5b58 100644 --- a/lib/run.cpp +++ b/lib/run.cpp @@ -1651,6 +1651,15 @@ bool runCrate(const Args &args, int argc, char** argv, int &outReturnCode) { std::string extraIface; // jail-side interface for this extra if (ex.mode == Spec::NetOptDetails::Mode::Bridge) { + // 1.1.24: validate the extra-interface bridge name like the + // primary bridge path (run.cpp ~1081). Without this, ex.bridgeIface + // flows straight into `ifconfig addm ` — a + // leading-'-' value is taken as an ifconfig option (argument + // injection). The primary path validated; this loop did not. + if (auto reason = BridgePure::validateBridgeName(ex.bridgeIface); + !reason.empty()) + ERR("extra[" << i << "] bridge mode: " << reason + << " (got '" << ex.bridgeIface << "')") RunNet::ensureBridgeModule(); auto bi = RunNet::createBridgeEpair(jid, jidStr, ex.bridgeIface, execInJail); extraIface = bi.ifaceB;