Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/foundation/subprocess.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/foundation/subprocess.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 28 additions & 4 deletions tests/test_subprocess.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 == '\\') {
Expand All @@ -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++;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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);
}
Loading