fix(net): reject control-char / path-traversal injection in wireguard/ipsec/migrate config fields (1.1.21)

CHANGELOG.md

@@ -6,6 +6,42 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ---
 
+## [1.1.21] — 2026-07-07
+
+**Security: reject control-char / path-traversal injection in
+config free-text fields (WireGuard, IPsec, migrate).**
+
+The pure-module audit found three sinks where a less-trusted string was
+emitted verbatim into a generated config or used as a filesystem path
+without a control-character / traversal check:
+
+- `WireguardPure::validateConfig` validated keys, CIDRs, ports and
+  endpoints but never the free-text `FwMark`, `DNS`, `MTU`, and peer
+  `description` fields. Each is written straight into the wg-quick
+  `.conf`; an embedded newline escapes the line and injects its own
+  directive — most dangerously `PostUp = <cmd>`, which wg-quick runs as
+  **root**. A new `hasControlChar` helper now rejects any byte `< 0x20`
+  or `== 0x7f` (incl. `\n`/`\r`) in those fields.
+
+- `IpsecPure::validateConfig` likewise never checked `leftid`,
+  `rightid`, or `description`, all emitted into `ipsec.conf`
+  (`leftid=…` / `rightid=…` / `# …`). A newline injects an arbitrary
+  conn option (e.g. `rightsubnet=0.0.0.0/0`, silently widening the
+  tunnel policy). Control bytes in those three fields are now rejected.
+
+- `MigratePure` gained `validateArtifactFile`: the artifact filename in
+  the migrate flow comes from the **source server's** `/export` JSON
+  (`extractFileField`) and is concatenated into `workDir + "/" + name`,
+  then used both as a `curl -o` target and an `unlink()` target. A
+  hostile or compromised source returning `"../../../etc/cron.d/pwn"`
+  got arbitrary file write **and** delete on the migrating host. The
+  filename is now validated as a single safe path component (no `/`, no
+  `..`, no control bytes, ≤ 255 chars) in `lib/migrate.cpp` before use.
+
+New `wireguard_pure_test`, `ipsec_pure_test`, and `migrate_pure_test`
+cases cover each rejection (newline / traversal / slash / control byte)
+and confirm clean values still validate.
+
 ## [1.1.20] — 2026-07-05
 
 **Fix: IPv6 lease parser didn't validate the jail-name charset.**

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.20" << std::endl;
+        std::cout << "crate 1.1.21" << 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.20" << std::endl;
+          std::cout << "crate 1.1.21" << std::endl;
           exit(0);
         default:
           err("unsupported short option '%s'", argv[a]);

docs/trust-model.md

@@ -4,7 +4,7 @@
 operators on one machine) and contributors extending the privileged
 surface.
 
-**Applies to:** 1.1.20 (rootless model + per-tenant authz series 1.1.12 →
+**Applies to:** 1.1.21 (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.20 means reasoning about who can reach
+Reasoning about isolation on 1.1.21 means reasoning about who can reach
 **privops**, not who can run `crate(1)`.
 
 ---

docs/trust-model.uk.md

@@ -4,7 +4,7 @@
 (кілька операторів на одній машині), і контрибʼютори, які розширюють
 привілейовану поверхню.
 
-**Стосується:** 1.1.20 (rootless-модель + серія per-tenant authz 1.1.12 →
+**Стосується:** 1.1.21 (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.20 — це міркувати про те, хто має доступ
+Міркувати про ізоляцію на 1.1.21 — це міркувати про те, хто має доступ
 до **privops**, а не хто може запустити `crate(1)`.
 
 ---

lib/ipsec_pure.cpp

@@ -225,6 +225,20 @@ std::string validateConfig(const std::vector<ConnSpec> &conns) {
     if (!c.autoStart.empty())
       if (auto e = validateAuto(c.autoStart); !e.empty())
         return prefixed(i, e);
+    // 1.1.21: leftid/rightid/description are emitted verbatim into
+    // ipsec.conf (as `leftid=…` / `rightid=…` / `# …`). A newline
+    // escapes the line and injects an arbitrary conn option (e.g.
+    // `rightsubnet=0.0.0.0/0`, silently widening the tunnel policy).
+    // validateConfig validated left/right but never these free-text
+    // fields.
+    for (auto &pr : {std::pair<const char*, const std::string&>{"leftid",  c.leftId},
+                     std::pair<const char*, const std::string&>{"rightid", c.rightId},
+                     std::pair<const char*, const std::string&>{"description", c.description}}) {
+      for (char ch : pr.second)
+        if (static_cast<unsigned char>(ch) < 0x20 ||
+            static_cast<unsigned char>(ch) == 0x7f)
+          return prefixed(i, std::string(pr.first) + " contains a control character");
+    }
   }
   return "";
 }

lib/migrate.cpp

@@ -164,6 +164,10 @@ bool migrateCommand(const Args &args) {
     r.artifactFile = extractFileField(exportBody);
     if (r.artifactFile.empty())
       ERR("export response did not include a 'file' field")
+    // 1.1.21: the filename is remote-controlled — validate before it is
+    // used as a `curl -o` / `unlink()` path (path-traversal guard).
+    if (auto e = MigratePure::validateArtifactFile(r.artifactFile); !e.empty())
+      ERR("export response 'file' field rejected: " << e)
 
     // Steps 2..5.
     for (auto &s : MigratePure::buildRemainingSteps(r))

lib/migrate_pure.cpp

@@ -177,6 +177,30 @@ std::string validateContainerName(const std::string &name) {
   return "";
 }
 
+// 1.1.21: the artifact filename comes from the SOURCE server's export
+// JSON (`extractFileField`), i.e. a remote, less-trusted input. It is
+// concatenated into `workDir + "/" + artifactFile` and used both as a
+// `curl -o` target and an `unlink()` target. Without validation a
+// hostile/compromised source could return `"../../../etc/cron.d/pwn"`
+// and get arbitrary file write AND delete on the migrating host. Treat
+// it as a single path COMPONENT: a plain filename, no slash, no `..`,
+// no control bytes.
+std::string validateArtifactFile(const std::string &name) {
+  if (name.empty()) return "artifact filename is empty";
+  if (name.size() > 255) return "artifact filename longer than 255 chars";
+  if (name == "." || name == "..") return "artifact filename is reserved";
+  for (char c : name) {
+    if (c == '/')
+      return "artifact filename must not contain '/' (path traversal)";
+    if (static_cast<unsigned char>(c) < 0x20 ||
+        static_cast<unsigned char>(c) == 0x7f)
+      return "artifact filename contains a control character";
+  }
+  if (name.find("..") != std::string::npos)
+    return "artifact filename must not contain '..'";
+  return "";
+}
+
 std::string normalizeBaseUrl(const std::string &endpoint) {
   bool hasScheme = false;
   auto rest = stripScheme(endpoint, &hasScheme);

lib/migrate_pure.h

@@ -50,6 +50,12 @@ std::string validateBearerToken(const std::string &token);
 // matches what a client can put in a URL path segment.
 std::string validateContainerName(const std::string &name);
 
+// 1.1.21: validate the artifact filename returned by the source
+// server's /export (a remote-controlled value) before it is used as a
+// filesystem path. Rejects empty, `/`, `..`, control bytes, > 255
+// chars — a single safe path component.
+std::string validateArtifactFile(const std::string &name);
+
 // --- Plan ---
 
 enum class StepKind {

lib/wireguard_pure.cpp

@@ -13,6 +13,18 @@ bool isBase64Char(char c) {
       || (c >= '0' && c <= '9') || c == '+' || c == '/';
 }
 
+// 1.1.21: a free-text field (DNS, FwMark, MTU, peer description) is
+// emitted verbatim into the wg-quick .conf. A newline would let it
+// inject its own directives — most dangerously `PostUp = <cmd>`, which
+// wg-quick runs as root. Reject any control byte (incl. \n/\r) and DEL.
+bool hasControlChar(const std::string &s) {
+  for (char c : s)
+    if (static_cast<unsigned char>(c) < 0x20 ||
+        static_cast<unsigned char>(c) == 0x7f)
+      return true;
+  return false;
+}
+
 bool isAlnum(char c) {
   return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z')
       || (c >= '0' && c <= '9');
@@ -180,6 +192,16 @@ std::string validateConfig(const InterfaceSpec &iface,
   if (!iface.listenPort.empty())
     if (auto e = validatePort(iface.listenPort); !e.empty())
       return "interface ListenPort: " + e;
+  // 1.1.21: fields emitted verbatim into the .conf must not carry a
+  // newline/control byte (would inject wg-quick directives like PostUp,
+  // which runs as root). validateConfig previously skipped these.
+  if (hasControlChar(iface.fwmark))
+    return "interface FwMark contains a control character";
+  for (auto &d : iface.dns)
+    if (hasControlChar(d))
+      return "interface DNS entry '" + d + "' contains a control character";
+  if (!iface.mtu.empty() && hasControlChar(iface.mtu.front()))
+    return "interface MTU contains a control character";
 
   // Peers
   if (peers.empty())
@@ -204,6 +226,12 @@ std::string validateConfig(const InterfaceSpec &iface,
         if (c < '0' || c > '9')
           return "peer #" + std::to_string(i + 1) + " PersistentKeepalive must be numeric";
     }
+    // 1.1.21: the peer description is rendered as a `# <description>`
+    // comment line — a newline escapes the comment and injects a
+    // directive into the [Interface]/[Peer] scope.
+    if (hasControlChar(p.description))
+      return "peer #" + std::to_string(i + 1) +
+             " description contains a control character";
   }
   return "";
 }

tests/unit/ipsec_pure_test.cpp

@@ -164,6 +164,41 @@ ATF_TEST_CASE_BODY(config_conn_index_in_error) {
   ATF_REQUIRE(err.find("conn #2") != std::string::npos);
 }
 
+// --- 1.1.21: injection hardening (control byte in id/description) ---
+
+ATF_TEST_CASE_WITHOUT_HEAD(config_leftid_newline_rejected);
+ATF_TEST_CASE_BODY(config_leftid_newline_rejected) {
+  auto c = goodConn();
+  c.leftId = "@vpn.example.com\n    rightsubnet=0.0.0.0/0";
+  auto err = validateConfig({c});
+  ATF_REQUIRE(err.find("leftid") != std::string::npos);
+}
+
+ATF_TEST_CASE_WITHOUT_HEAD(config_rightid_newline_rejected);
+ATF_TEST_CASE_BODY(config_rightid_newline_rejected) {
+  auto c = goodConn();
+  c.rightId = "peer\n    auto=start";
+  auto err = validateConfig({c});
+  ATF_REQUIRE(err.find("rightid") != std::string::npos);
+}
+
+ATF_TEST_CASE_WITHOUT_HEAD(config_description_newline_rejected);
+ATF_TEST_CASE_BODY(config_description_newline_rejected) {
+  auto c = goodConn();
+  c.description = "tunnel\nconn evil";
+  auto err = validateConfig({c});
+  ATF_REQUIRE(err.find("description") != std::string::npos);
+}
+
+ATF_TEST_CASE_WITHOUT_HEAD(config_clean_id_and_description_accepted);
+ATF_TEST_CASE_BODY(config_clean_id_and_description_accepted) {
+  auto c = goodConn();
+  c.leftId = "@vpn.example.com";
+  c.rightId = "C=UA, O=Example, CN=peer";
+  c.description = "DC1 to DC2";
+  ATF_REQUIRE_EQ(validateConfig({c}), std::string());
+}
+
 // --- renderConf ---
 
 ATF_TEST_CASE_WITHOUT_HEAD(render_emits_setup_and_conn);
@@ -244,6 +279,10 @@ ATF_INIT_TEST_CASES(tcs) {
   ATF_ADD_TEST_CASE(tcs, config_no_conns_rejected);
   ATF_ADD_TEST_CASE(tcs, config_keyexchange_invalid_rejected);
   ATF_ADD_TEST_CASE(tcs, config_conn_index_in_error);
+  ATF_ADD_TEST_CASE(tcs, config_leftid_newline_rejected);
+  ATF_ADD_TEST_CASE(tcs, config_rightid_newline_rejected);
+  ATF_ADD_TEST_CASE(tcs, config_description_newline_rejected);
+  ATF_ADD_TEST_CASE(tcs, config_clean_id_and_description_accepted);
   ATF_ADD_TEST_CASE(tcs, render_emits_setup_and_conn);
   ATF_ADD_TEST_CASE(tcs, render_csv_joins_subnets);
   ATF_ADD_TEST_CASE(tcs, render_omits_optional_fields_when_empty);

tests/unit/migrate_pure_test.cpp

@@ -15,6 +15,7 @@ using MigratePure::describeStep;
 using MigratePure::normalizeBaseUrl;
 using MigratePure::redactToken;
 using MigratePure::validateBearerToken;
+using MigratePure::validateArtifactFile;
 using MigratePure::validateContainerName;
 using MigratePure::validateEndpoint;
 
@@ -83,6 +84,30 @@ ATF_TEST_CASE_BODY(name_invalid_rejected) {
   ATF_REQUIRE(!validateContainerName("foo;rm").empty());
 }
 
+// --- 1.1.21: validateArtifactFile (remote-controlled filename) ---
+
+ATF_TEST_CASE_WITHOUT_HEAD(artifact_file_typical_accepted);
+ATF_TEST_CASE_BODY(artifact_file_typical_accepted) {
+  ATF_REQUIRE_EQ(validateArtifactFile("web01-20260707.crate"), std::string());
+  ATF_REQUIRE_EQ(validateArtifactFile("export_1.tar.zst"), std::string());
+}
+
+ATF_TEST_CASE_WITHOUT_HEAD(artifact_file_traversal_rejected);
+ATF_TEST_CASE_BODY(artifact_file_traversal_rejected) {
+  // A hostile/compromised source server returns these; they must be
+  // refused before they become a `curl -o` / `unlink()` path.
+  ATF_REQUIRE(!validateArtifactFile("").empty());
+  ATF_REQUIRE(!validateArtifactFile(".").empty());
+  ATF_REQUIRE(!validateArtifactFile("..").empty());
+  ATF_REQUIRE(!validateArtifactFile("../../etc/cron.d/pwn").empty());
+  ATF_REQUIRE(!validateArtifactFile("sub/dir/file").empty());
+  ATF_REQUIRE(!validateArtifactFile("/etc/passwd").empty());
+  ATF_REQUIRE(!validateArtifactFile("a..b").empty());   // any ".." substring
+  ATF_REQUIRE(!validateArtifactFile("bad\nname").empty());
+  ATF_REQUIRE(!validateArtifactFile("bad\tname").empty());
+  ATF_REQUIRE(!validateArtifactFile(std::string(256, 'a')).empty());
+}
+
 // --- normalizeBaseUrl ---
 
 ATF_TEST_CASE_WITHOUT_HEAD(normalize_adds_https_when_missing);
@@ -204,6 +229,8 @@ ATF_INIT_TEST_CASES(tcs) {
   ATF_ADD_TEST_CASE(tcs, token_invalid_rejected);
   ATF_ADD_TEST_CASE(tcs, name_typical_accepted);
   ATF_ADD_TEST_CASE(tcs, name_invalid_rejected);
+  ATF_ADD_TEST_CASE(tcs, artifact_file_typical_accepted);
+  ATF_ADD_TEST_CASE(tcs, artifact_file_traversal_rejected);
   ATF_ADD_TEST_CASE(tcs, normalize_adds_https_when_missing);
   ATF_ADD_TEST_CASE(tcs, normalize_preserves_existing_scheme);
   ATF_ADD_TEST_CASE(tcs, build_export_step_shape);

tests/unit/wireguard_pure_test.cpp

@@ -178,6 +178,51 @@ ATF_TEST_CASE_BODY(config_peer_index_in_error) {
   ATF_REQUIRE(err.find("peer #2") != std::string::npos);
 }
 
+// --- 1.1.21: injection hardening (control byte in free-text fields) ---
+
+ATF_TEST_CASE_WITHOUT_HEAD(config_iface_fwmark_newline_rejected);
+ATF_TEST_CASE_BODY(config_iface_fwmark_newline_rejected) {
+  auto i = goodIface();
+  i.fwmark = "0x1\nPostUp = touch /tmp/pwn";
+  auto err = validateConfig(i, {goodPeer()});
+  ATF_REQUIRE(err.find("FwMark") != std::string::npos);
+}
+
+ATF_TEST_CASE_WITHOUT_HEAD(config_iface_dns_newline_rejected);
+ATF_TEST_CASE_BODY(config_iface_dns_newline_rejected) {
+  auto i = goodIface();
+  i.dns = {"1.1.1.1", "9.9.9.9\nPostUp = id"};
+  auto err = validateConfig(i, {goodPeer()});
+  ATF_REQUIRE(err.find("DNS") != std::string::npos);
+}
+
+ATF_TEST_CASE_WITHOUT_HEAD(config_iface_mtu_newline_rejected);
+ATF_TEST_CASE_BODY(config_iface_mtu_newline_rejected) {
+  auto i = goodIface();
+  i.mtu = {"1420\nPostUp = id"};
+  auto err = validateConfig(i, {goodPeer()});
+  ATF_REQUIRE(err.find("MTU") != std::string::npos);
+}
+
+ATF_TEST_CASE_WITHOUT_HEAD(config_peer_description_newline_rejected);
+ATF_TEST_CASE_BODY(config_peer_description_newline_rejected) {
+  auto p = goodPeer();
+  p.description = "edge\nPostUp = touch /tmp/pwn";
+  auto err = validateConfig(goodIface(), {p});
+  ATF_REQUIRE(err.find("description") != std::string::npos);
+}
+
+ATF_TEST_CASE_WITHOUT_HEAD(config_clean_free_text_accepted);
+ATF_TEST_CASE_BODY(config_clean_free_text_accepted) {
+  auto i = goodIface();
+  i.fwmark = "0x1";
+  i.dns = {"1.1.1.1"};
+  i.mtu = {"1420"};
+  auto p = goodPeer();
+  p.description = "edge router (site B)";
+  ATF_REQUIRE_EQ(validateConfig(i, {p}), std::string());
+}
+
 // --- renderConf ---
 
 ATF_TEST_CASE_WITHOUT_HEAD(render_emits_interface_section);
@@ -273,6 +318,11 @@ ATF_INIT_TEST_CASES(tcs) {
   ATF_ADD_TEST_CASE(tcs, config_no_peers_rejected);
   ATF_ADD_TEST_CASE(tcs, config_no_addresses_rejected);
   ATF_ADD_TEST_CASE(tcs, config_peer_index_in_error);
+  ATF_ADD_TEST_CASE(tcs, config_iface_fwmark_newline_rejected);
+  ATF_ADD_TEST_CASE(tcs, config_iface_dns_newline_rejected);
+  ATF_ADD_TEST_CASE(tcs, config_iface_mtu_newline_rejected);
+  ATF_ADD_TEST_CASE(tcs, config_peer_description_newline_rejected);
+  ATF_ADD_TEST_CASE(tcs, config_clean_free_text_accepted);
   ATF_ADD_TEST_CASE(tcs, render_emits_interface_section);
   ATF_ADD_TEST_CASE(tcs, render_csv_joins_addresses);
   ATF_ADD_TEST_CASE(tcs, render_omits_optional_listenport);