diff --git a/src/foundation/subprocess.c b/src/foundation/subprocess.c index 61730c24..cbcea28a 100644 --- a/src/foundation/subprocess.c +++ b/src/foundation/subprocess.c @@ -135,8 +135,8 @@ static bool cbm_tail_log(const char *log_file, long *tail_pos, cbm_proc_log_cb c /* ── Windows command-line quoting (pure; unit-tested on every platform) ─────── */ /* Append char `c` to buf[cap], reserving the final byte for a NUL terminator. - * Sets *ovf on overflow and stops writing; pos keeps advancing so the caller - * still detects the overflow after the loop. */ + * On overflow: sets *ovf, stops writing, and returns pos UNCHANGED — callers detect + * the overflow via the *ovf flag (not via the return value). */ static size_t cbm_cmdline_put(char *buf, size_t cap, size_t pos, char c, bool *ovf) { if (pos + 1 >= cap) { *ovf = true; @@ -213,6 +213,7 @@ bool cbm_build_win_cmdline(char *buf, size_t cap, const char *const *argv) { for (int i = 0; argv[i]; i++) { pos = cbm_cmdline_append_arg(buf, cap, pos, argv[i], i == 0, &ovf); if (ovf) { + buf[0] = '\0'; /* overflow: leave buf a valid (empty) string, never unterminated */ return false; } } diff --git a/src/foundation/subprocess.h b/src/foundation/subprocess.h index 826055b5..b2a87637 100644 --- a/src/foundation/subprocess.h +++ b/src/foundation/subprocess.h @@ -77,7 +77,8 @@ const char *cbm_proc_outcome_str(cbm_proc_outcome_t o); /* Build a Windows CreateProcess command line from a NULL-terminated argv, applying * the Microsoft C runtime quoting rules (quote-wrap + escape embedded quotes and * their preceding backslashes) so the spawned child re-parses byte-identical argv. - * Returns true on success, false on overflow (buf then holds a truncated string). + * Returns true on success, false on overflow (on overflow buf is set to an empty + * string, never left unterminated). * * CreateProcess re-parses a SINGLE command string into argv, so a naive `"%s"` wrap * silently corrupts any element containing a double-quote — e.g. the index worker's diff --git a/tests/test_subprocess.c b/tests/test_subprocess.c index d4138187..c7dad2fc 100644 --- a/tests/test_subprocess.c +++ b/tests/test_subprocess.c @@ -195,6 +195,15 @@ static int parse_win_cmdline(const char *cmd, char out[][256], int max_args) { char *o = out[argc]; size_t oi = 0; bool in_quotes = false; + /* Guard every write: each out[] row is 256 bytes; test args stay well under + * that, but cap defensively so a future longer arg fails a length assertion + * rather than smashing the stack. */ +#define PUTO(ch) \ + do { \ + if (oi < 255) { \ + o[oi++] = (ch); \ + } \ + } while (0) for (;;) { size_t nbs = 0; while (*p == '\\') { @@ -203,24 +212,26 @@ static int parse_win_cmdline(const char *cmd, char out[][256], int max_args) { } if (*p == '"') { for (size_t k = 0; k < nbs / 2; k++) { - o[oi++] = '\\'; + PUTO('\\'); } if (nbs % 2) { - o[oi++] = '"'; /* odd run → the quote is an escaped literal */ + PUTO('"'); /* odd run → the quote is an escaped literal */ } else { in_quotes = !in_quotes; /* even run → the quote is a delimiter */ } p++; } else { for (size_t k = 0; k < nbs; k++) { - o[oi++] = '\\'; + PUTO('\\'); } if (*p == '\0' || (!in_quotes && (*p == ' ' || *p == '\t'))) { break; } - o[oi++] = *p++; + PUTO(*p); + p++; } } +#undef PUTO o[oi] = '\0'; argc++; } @@ -293,6 +304,18 @@ TEST(win_cmdline_overflow_rejected) { PASS(); } +/* Reproduce-first guard for the overflow CONTRACT (subprocess.h): on overflow buf + * must be left a valid (empty) string. RED on the pre-fix code — the overflow path + * returned without terminating buf, so buf[0] held the first quoted byte ('"') — + * and GREEN once the overflow path sets buf[0] = '\0'. */ +TEST(win_cmdline_overflow_leaves_empty_string) { + char buf[8]; + const char *const argv[] = {"averylongprogramname", "x", NULL}; + ASSERT_FALSE(cbm_build_win_cmdline(buf, sizeof(buf), argv)); /* overflows cap */ + ASSERT_EQ(buf[0], '\0'); /* pre-fix: buf[0] == '"' (a partial byte), not NUL */ + PASS(); +} + SUITE(subprocess) { RUN_TEST(subprocess_classify_clean); RUN_TEST(subprocess_classify_exit_nonzero); @@ -310,4 +333,5 @@ SUITE(subprocess) { RUN_TEST(win_cmdline_index_worker_json); RUN_TEST(win_cmdline_roundtrip_battery); RUN_TEST(win_cmdline_overflow_rejected); + RUN_TEST(win_cmdline_overflow_leaves_empty_string); }