From 766e3c89c0c70b5a381ebe76e61b2b528cb711b5 Mon Sep 17 00:00:00 2001 From: David Connolly Date: Sat, 13 Jun 2026 02:23:56 +0100 Subject: [PATCH 1/7] Make the device load and run on the Windows NT 3.1 floor NT 3.1 (the native-Win32 floor, July 1993) rejected the binary at three points the Win32s/Win9x tiers structurally never surfaced, each found by on-target QEMU validation: - The PE was stamped OS/subsystem version 4.0 (mingw default); NT 3.1's loader refuses a newer stamp with ERROR_BAD_FORMAT. Stamp 3.10 in the link step - still loads on every later Windows, which accept older stamps. - SetHandleInformation (NT 3.51/Win95) was a static kernel32 import in exec_ops.c. Resolve it through the feat.c capability probe, with the classic DuplicateHandle non-inheritable-copy fallback for NT 3.1. - lstrcpynA is absent from NT 3.1's kernel32 (only lstrcpy/A/W exist). Supply McpStrCpyN (strutil.c), a DBCS-aware bounded copy, and drop the static import across the call sites. mcp-w32s.exe still imports only kernel32 + user32, stays i386/console, and the full build plus every string-helper-affected suite is green (the two exec/serial suites fail identically with and without this change - the known wine-vs-native divergence, not a regression). Verified live: the device now loads and runs on real NT 3.1 Advanced Server under QEMU. The remaining serial-port-open issue is tracked separately in the host plan. Co-Authored-By: Claude Opus 4.8 --- CMakeLists.txt | 30 ++++++++++++++++++---------- src/audit.c | 3 ++- src/catalog.c | 17 ++++++++-------- src/exec_ops.c | 35 +++++++++++++++++++++++++++++---- src/feat.c | 7 +++++++ src/feat.h | 2 ++ src/mcp-w32s.c | 15 +++++++------- src/mem_ops.c | 53 +++++++++++++++++++++++++------------------------- src/serial.c | 5 +++-- src/strutil.c | 41 ++++++++++++++++++++++++++++++++++++++ src/strutil.h | 23 ++++++++++++++++++++++ src/tcp.c | 13 +++++++------ src/uart.c | 5 +++-- 13 files changed, 183 insertions(+), 66 deletions(-) create mode 100644 src/strutil.c create mode 100644 src/strutil.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 0b35b14..9a8f493 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -67,10 +67,19 @@ endif() # Win32s load requirements: the PE must carry relocations (/FIXED:NO) and be # based at 0x10000 (the default 0x400000 region is occupied by Win32s). # Applied to every executable we emit. +# +# PE version stamp: the native-Win32 floor is Windows NT 3.1 (1993), whose loader +# rejects a PE stamped with a newer OS/subsystem version (ERROR_BAD_FORMAT) - and +# mingw/MSVC default the stamp to 4.0. Stamp 3.10 so the 1993 loader accepts it; +# 3.10 still loads on every later Windows (they accept older stamps). The Win32s/ +# Win9x loaders do not enforce this, so only NT 3.1 surfaced it (see the host +# repo's plan/PHASE6.md, NT 3.1 floor validation). if(MSVC) - add_link_options(/FIXED:NO /BASE:0x10000 /MACHINE:I386 /SUBSYSTEM:CONSOLE) + add_link_options(/FIXED:NO /BASE:0x10000 /MACHINE:I386 /SUBSYSTEM:CONSOLE,3.10) else() - add_link_options(-Wl,--dynamicbase -Wl,--image-base,0x10000) + add_link_options(-Wl,--dynamicbase -Wl,--image-base,0x10000 + -Wl,--major-os-version,3 -Wl,--minor-os-version,10 + -Wl,--major-subsystem-version,3 -Wl,--minor-subsystem-version,10) endif() include_directories(src tests) @@ -96,7 +105,8 @@ set(CORE_SOURCES src/pty_exec.c src/toolchain_probe.c src/mem_ops.c - src/audit.c) + src/audit.c + src/strutil.c) # --- main executable ------------------------------------------------------- # The TCP backend is runtime-loaded (LoadLibraryA), so we deliberately do NOT @@ -142,7 +152,7 @@ target_link_libraries(test_transport PRIVATE kernel32) # test_tcp's own client side links wsock32 directly; the backend under test # still resolves Winsock at runtime. -add_executable(test_tcp tests/test_tcp.c src/tcp.c src/transport.c) +add_executable(test_tcp tests/test_tcp.c src/tcp.c src/transport.c src/strutil.c) target_link_libraries(test_tcp PRIVATE kernel32 wsock32) # test_serial includes mcp-w32s.c with TEST_BUILD to exclude main() and reach @@ -182,14 +192,14 @@ add_executable(test_binfmt tests/test_binfmt.c src/binfmt.c src/feat.c) target_link_libraries(test_binfmt PRIVATE kernel32) # catalog.c serialises the listCommands listing via json_parser.c's JsonEscape. -add_executable(test_catalog tests/test_catalog.c src/catalog.c src/json_parser.c) +add_executable(test_catalog tests/test_catalog.c src/catalog.c src/json_parser.c src/strutil.c) target_link_libraries(test_catalog PRIVATE kernel32) # exec_ops.c widens the cmdline for the -W spawn (encoding.c), so the # standalone exec target links the codec + tables too. add_executable(test_exec_ops tests/test_exec_ops.c src/exec_ops.c src/feat.c src/binfmt.c - src/encoding.c src/charset_tables.c src/charset_tables_data.c) + src/encoding.c src/charset_tables.c src/charset_tables_data.c src/strutil.c) target_link_libraries(test_exec_ops PRIVATE kernel32) add_executable(test_pty_exec tests/test_pty_exec.c src/pty_exec.c src/feat.c) @@ -199,7 +209,7 @@ target_link_libraries(test_pty_exec PRIVATE kernel32) add_executable(test_toolchain_probe tests/test_toolchain_probe.c src/toolchain_probe.c src/exec_ops.c src/catalog.c src/json_parser.c src/feat.c src/binfmt.c - src/encoding.c src/charset_tables.c src/charset_tables_data.c) + src/encoding.c src/charset_tables.c src/charset_tables_data.c src/strutil.c) target_link_libraries(test_toolchain_probe PRIVATE kernel32) # --- 5.3 memory peek/poke test targets ------------------------------------- @@ -208,7 +218,7 @@ target_link_libraries(test_toolchain_probe PRIVATE kernel32) # helper child next to it. audit.c is linked because MemPoke audits. add_executable(test_mem_ops tests/test_mem_ops.c src/mem_ops.c src/audit.c - src/feat.c src/catalog.c src/json_parser.c src/argv.c) + src/feat.c src/catalog.c src/json_parser.c src/argv.c src/strutil.c) target_compile_definitions(test_mem_ops PRIVATE TEST_BUILD) target_link_libraries(test_mem_ops PRIVATE kernel32 user32) @@ -217,7 +227,7 @@ add_executable(mem_target tests/mem_target.c) target_link_libraries(mem_target PRIVATE kernel32) add_dependencies(test_mem_ops mem_target) -add_executable(test_audit tests/test_audit.c src/audit.c) +add_executable(test_audit tests/test_audit.c src/audit.c src/strutil.c) target_link_libraries(test_audit PRIVATE kernel32 user32) # --- 5.4 text-encoding test targets ---------------------------------------- @@ -242,7 +252,7 @@ target_link_libraries(test_pbt_encoding PRIVATE kernel32) # test_uart: the prop.h on-target mirror of the theft UART properties, proving # the pure detection ladder + driving logic hold on the shipped C89/i386 path. # UART_HOST_PURE excludes the asm IN/OUT seam + transport wiring (links uart.c only). -add_executable(test_uart tests/test_uart.c src/uart.c) +add_executable(test_uart tests/test_uart.c src/uart.c src/strutil.c) target_compile_definitions(test_uart PRIVATE UART_HOST_PURE) target_link_libraries(test_uart PRIVATE kernel32) diff --git a/src/audit.c b/src/audit.c index 5b2ce78..603cdb8 100644 --- a/src/audit.c +++ b/src/audit.c @@ -13,6 +13,7 @@ */ #include +#include "strutil.h" #include "audit.h" #define AUDIT_DEFAULT_NAME "audit-mem.log" @@ -78,7 +79,7 @@ int AuditIsWritable(void) int AuditConfigure(int armRequested, const char *path) { if (path != NULL && path[0] != '\0') { - lstrcpynA(g_auditPath, path, (int)sizeof(g_auditPath)); + McpStrCpyN(g_auditPath, path, (int)sizeof(g_auditPath)); } else { resolve_default_path(g_auditPath, (int)sizeof(g_auditPath)); } diff --git a/src/catalog.c b/src/catalog.c index 5023b8e..1baf0ae 100644 --- a/src/catalog.c +++ b/src/catalog.c @@ -13,6 +13,7 @@ */ #include +#include "strutil.h" #include #include #include "catalog.h" @@ -57,7 +58,7 @@ typedef struct { static void scanError(Scanner *s, const char *msg) { if (s->err != NULL && s->errSize > 0) { - lstrcpynA(s->err, msg, s->errSize); + McpStrCpyN(s->err, msg, s->errSize); } } @@ -448,7 +449,7 @@ static int parseCommands(Scanner *s, Catalog *cat) s->p++; if (cat->entry_count < CATALOG_MAX_ENTRIES) { CatalogEntry *e = &cat->entries[cat->entry_count]; - lstrcpynA(e->name, name, sizeof(e->name)); + McpStrCpyN(e->name, name, sizeof(e->name)); if (!parseEntry(s, e)) { return 0; } @@ -573,7 +574,7 @@ int CatalogLoad(const char *path, Catalog **outCat, char *errMsg, int errSize) fileBuf = (char *)malloc(CATALOG_FILE_MAX); if (fileBuf == NULL) { if (errMsg != NULL) { - lstrcpynA(errMsg, "out of memory", errSize); + McpStrCpyN(errMsg, "out of memory", errSize); } return 0; } @@ -582,7 +583,7 @@ int CatalogLoad(const char *path, Catalog **outCat, char *errMsg, int errSize) if (len < 0) { free(fileBuf); if (errMsg != NULL) { - lstrcpynA(errMsg, "catalog file not found or unreadable", errSize); + McpStrCpyN(errMsg, "catalog file not found or unreadable", errSize); } return 0; } @@ -591,7 +592,7 @@ int CatalogLoad(const char *path, Catalog **outCat, char *errMsg, int errSize) if (cat == NULL) { free(fileBuf); if (errMsg != NULL) { - lstrcpynA(errMsg, "out of memory", errSize); + McpStrCpyN(errMsg, "out of memory", errSize); } return 0; } @@ -615,7 +616,7 @@ int CatalogLoad(const char *path, Catalog **outCat, char *errMsg, int errSize) free(fileBuf); free(cat); if (errMsg != NULL) { - lstrcpynA(errMsg, "no commands in catalog", errSize); + McpStrCpyN(errMsg, "no commands in catalog", errSize); } return 0; } @@ -702,7 +703,7 @@ int CatalogValidateArgs(const CatalogEntry *entry, const char **argv, } if (entry == NULL) { if (errMsg != NULL) { - lstrcpynA(errMsg, "no catalog entry", errSize); + McpStrCpyN(errMsg, "no catalog entry", errSize); } return 0; } @@ -717,7 +718,7 @@ int CatalogValidateArgs(const CatalogEntry *entry, const char **argv, const CatalogOption *opt = findOption(entry, tok); if (opt == NULL) { if (errMsg != NULL) { - lstrcpynA(errMsg, "argument not allowed", errSize); + McpStrCpyN(errMsg, "argument not allowed", errSize); } return 0; } diff --git a/src/exec_ops.c b/src/exec_ops.c index 0463442..ea5ee6d 100644 --- a/src/exec_ops.c +++ b/src/exec_ops.c @@ -37,6 +37,7 @@ #include "feat.h" #include "binfmt.h" #include "encoding.h" /* Utf8ToUtf16 for the -W spawn (wide tier) */ +#include "strutil.h" /* McpStrCpyN (the NT 3.1 floor lacks lstrcpynA) */ /* ------------------------------------------------------------------ * Job-object declarations. MinGW's C89 headers may lack these; declare @@ -124,7 +125,33 @@ typedef struct { static void SetMsg(char *errMsg, int errSize, const char *s) { if (errMsg != NULL && errSize > 0) { - lstrcpynA(errMsg, s, errSize); + McpStrCpyN(errMsg, s, errSize); + } +} + +/* + * ClearHandleInherit - clear a handle's HANDLE_FLAG_INHERIT (Q5: the + * parent-only pipe ends must not be inherited by the child). NT 3.1's kernel32 + * - the native-Win32 floor - has no SetHandleInformation (it arrived in NT + * 3.51/Win95), so when the runtime probe found it absent we fall back to the + * classic pre-3.51 idiom: duplicate the handle non-inheritable and drop the + * inheritable original. DuplicateHandle is present since NT 3.1. + */ +static void ClearHandleInherit(HANDLE *ph) +{ + HANDLE dup; + if (ph == NULL || *ph == NULL || *ph == INVALID_HANDLE_VALUE) { + return; + } + if (g_features.pSetHandleInformation != NULL) { + g_features.pSetHandleInformation(*ph, HANDLE_FLAG_INHERIT, 0); + return; + } + if (DuplicateHandle(GetCurrentProcess(), *ph, + GetCurrentProcess(), &dup, + 0, FALSE, DUPLICATE_SAME_ACCESS)) { + CloseHandle(*ph); + *ph = dup; } } @@ -375,9 +402,9 @@ int ExecOpRun( goto fail_pipes; } /* Q5: child must not inherit the parent-only ends. */ - SetHandleInformation(inWr, HANDLE_FLAG_INHERIT, 0); - SetHandleInformation(outRd, HANDLE_FLAG_INHERIT, 0); - SetHandleInformation(errRd, HANDLE_FLAG_INHERIT, 0); + ClearHandleInherit(&inWr); + ClearHandleInherit(&outRd); + ClearHandleInherit(&errRd); memset(&si, 0, sizeof(si)); si.cb = sizeof(si); diff --git a/src/feat.c b/src/feat.c index 18fa1df..3c60a31 100644 --- a/src/feat.c +++ b/src/feat.c @@ -117,6 +117,13 @@ void FeatInit(void) g_features.has_get_binary_type = 1; } + /* Absent on the NT 3.1 floor; callers fall back to DuplicateHandle. */ + proc = GetProcAddress(hKernel, "SetHandleInformation"); + if (proc != NULL) { + g_features.pSetHandleInformation = + (BOOL (WINAPI *)(HANDLE, DWORD, DWORD))proc; + } + proc = GetProcAddress(hKernel, "IsWow64Process"); if (proc != NULL) { g_features.pIsWow64Process = diff --git a/src/feat.h b/src/feat.h index a98bd84..018861c 100644 --- a/src/feat.h +++ b/src/feat.h @@ -69,6 +69,8 @@ typedef struct { BOOL (WINAPI *pAssignProcessToJobObject)(HANDLE, HANDLE); BOOL (WINAPI *pSetInformationJobObject)(HANDLE, int, LPVOID, DWORD); BOOL (WINAPI *pGetBinaryTypeA)(LPCSTR, LPDWORD); + /* NT 3.1's kernel32 lacks SetHandleInformation (arrived NT 3.51/Win95). */ + BOOL (WINAPI *pSetHandleInformation)(HANDLE, DWORD, DWORD); BOOL (WINAPI *pIsWow64Process)(HANDLE, BOOL *); BOOL (WINAPI *pGenerateConsoleCtrlEvent)(DWORD, DWORD); BOOL (WINAPI *pQueryFullProcessImageNameA)(HANDLE, DWORD, LPSTR, LPDWORD); diff --git a/src/mcp-w32s.c b/src/mcp-w32s.c index 2ce7be0..33606e2 100644 --- a/src/mcp-w32s.c +++ b/src/mcp-w32s.c @@ -14,6 +14,7 @@ */ #include +#include "strutil.h" #include #include "common.h" #include "json_parser.h" @@ -77,7 +78,7 @@ void ExecInjectOrphanForTest(HANDLE h, DWORD startTick, const char *cmdLine) { g_orphanHandle = h; g_orphanStartTick = startTick; - lstrcpynA(g_orphanCmdLine, cmdLine != NULL ? cmdLine : "", MCP_MAX_LINE); + McpStrCpyN(g_orphanCmdLine, cmdLine != NULL ? cmdLine : "", MCP_MAX_LINE); } #endif @@ -275,7 +276,7 @@ static void HandleExec(JsonCommand *cmd, Transport *t, int isPty) /* 2. Command name: argv[0] preferred, first token of line legacy. */ name[0] = '\0'; if (cmd->argv_count > 0) { - lstrcpynA(name, cmd->argv[0], (int)sizeof(name)); + McpStrCpyN(name, cmd->argv[0], (int)sizeof(name)); } else { int n; n = 0; @@ -383,7 +384,7 @@ static void HandleExec(JsonCommand *cmd, Transport *t, int isPty) return; } } else { - lstrcpynA(joined, cmd->line, (int)sizeof(joined)); + McpStrCpyN(joined, cmd->line, (int)sizeof(joined)); } if (viaShell) { @@ -405,7 +406,7 @@ static void HandleExec(JsonCommand *cmd, Transport *t, int isPty) prefix = g_features.is_win32s ? "command.com /c" : "cmd.exe /c"; } - lstrcpynA(cmdLine, prefix, (int)sizeof(cmdLine)); + McpStrCpyN(cmdLine, prefix, (int)sizeof(cmdLine)); if (escapedTail[0] != '\0') { if (lstrlenA(cmdLine) + 1 + lstrlenA(escapedTail) >= (int)sizeof(cmdLine)) { @@ -416,7 +417,7 @@ static void HandleExec(JsonCommand *cmd, Transport *t, int isPty) lstrcatA(cmdLine, escapedTail); } } else { - lstrcpynA(cmdLine, joined, (int)sizeof(cmdLine)); + McpStrCpyN(cmdLine, joined, (int)sizeof(cmdLine)); } } @@ -529,7 +530,7 @@ static void HandleExec(JsonCommand *cmd, Transport *t, int isPty) if (res.still_active) { g_orphanHandle = res.orphan_handle; g_orphanStartTick = res.orphan_start_tick; - lstrcpynA(g_orphanCmdLine, cmdLine, MCP_MAX_LINE); + McpStrCpyN(g_orphanCmdLine, cmdLine, MCP_MAX_LINE); send_exec_error(cmd->id, "timed out", t); return; } @@ -1043,7 +1044,7 @@ static Catalog *LoadCatalogAtStartup(const TransportConfig *config) char err[160]; if (config->catalogPath[0] != '\0') { - lstrcpynA(path, config->catalogPath, (int)sizeof(path)); + McpStrCpyN(path, config->catalogPath, (int)sizeof(path)); } else { int n; char *p; diff --git a/src/mem_ops.c b/src/mem_ops.c index 8525d5c..49f7b95 100644 --- a/src/mem_ops.c +++ b/src/mem_ops.c @@ -122,6 +122,7 @@ int MemRangeInBounds(unsigned long addr, unsigned long len, unsigned long cap) #ifndef MEM_OPS_HOST_PURE #include +#include "strutil.h" #include "mem_ops.h" #include "feat.h" #include "audit.h" @@ -351,7 +352,7 @@ void MemSpawnRetain(const Catalog *cat, int unsafeMode, out->reason[0] = '\0'; if (argv == NULL || argc <= 0 || argv[0] == NULL || argv[0][0] == '\0') { - lstrcpynA(out->reason, "empty command", (int)sizeof(out->reason)); + McpStrCpyN(out->reason, "empty command", (int)sizeof(out->reason)); return; } @@ -361,12 +362,12 @@ void MemSpawnRetain(const Catalog *cat, int unsafeMode, if (unsafeMode == 0 && cat != NULL) { entry = CatalogLookup(cat, argv[0]); if (entry == NULL) { - lstrcpynA(out->reason, "command not in catalog", + McpStrCpyN(out->reason, "command not in catalog", (int)sizeof(out->reason)); return; } if (CatalogEntryIsBuiltin(entry)) { - lstrcpynA(out->reason, "shell builtin is not a spawn-retain target", + McpStrCpyN(out->reason, "shell builtin is not a spawn-retain target", (int)sizeof(out->reason)); return; } @@ -381,13 +382,13 @@ void MemSpawnRetain(const Catalog *cat, int unsafeMode, } } if (slot == NULL) { - lstrcpynA(out->reason, "process table full", (int)sizeof(out->reason)); + McpStrCpyN(out->reason, "process table full", (int)sizeof(out->reason)); return; } /* Build the command line from argv (CreateProcessA quoting). */ if (ArgvJoin(argv, argc, cmdLine, (int)sizeof(cmdLine)) < 0) { - lstrcpynA(out->reason, "command line too long", + McpStrCpyN(out->reason, "command line too long", (int)sizeof(out->reason)); return; } @@ -421,9 +422,9 @@ void MemSpawnRetain(const Catalog *cat, int unsafeMode, slot->in_use = 1; slot->handle = pi.hProcess; slot->pid = (int)pi.dwProcessId; - lstrcpynA(slot->command, cmdLine, (int)sizeof(slot->command)); + McpStrCpyN(slot->command, cmdLine, (int)sizeof(slot->command)); - lstrcpynA(out->token, slot->token, (int)sizeof(out->token)); + McpStrCpyN(out->token, slot->token, (int)sizeof(out->token)); out->pid = slot->pid; out->ok = 1; } @@ -503,7 +504,7 @@ void MemPeek(const char *token, unsigned long addr, unsigned long len, res->reason[0] = '\0'; if (!MemRangeInBounds(addr, len, MEM_MAX_ACCESS)) { - lstrcpynA(res->reason, "range out of bounds", (int)sizeof(res->reason)); + McpStrCpyN(res->reason, "range out of bounds", (int)sizeof(res->reason)); return; } @@ -513,7 +514,7 @@ void MemPeek(const char *token, unsigned long addr, unsigned long len, /* A device with no memory tier never reads (defence in depth: the bridge * prunes the tools, but a direct wire client must be refused too). */ if (tier == MEM_TIER_NONE) { - lstrcpynA(res->reason, "no memory capability", (int)sizeof(res->reason)); + McpStrCpyN(res->reason, "no memory capability", (int)sizeof(res->reason)); return; } @@ -526,17 +527,17 @@ void MemPeek(const char *token, unsigned long addr, unsigned long len, FeatSizeT got; if (token == NULL || token[0] == '\0') { - lstrcpynA(res->reason, "token required on the process tier", + McpStrCpyN(res->reason, "token required on the process tier", (int)sizeof(res->reason)); return; } h = MemTokenHandle(token); if (h == NULL) { - lstrcpynA(res->reason, "invalid token", (int)sizeof(res->reason)); + McpStrCpyN(res->reason, "invalid token", (int)sizeof(res->reason)); return; } if (g_pReadProcessMemory == NULL) { - lstrcpynA(res->reason, "ReadProcessMemory unavailable", + McpStrCpyN(res->reason, "ReadProcessMemory unavailable", (int)sizeof(res->reason)); return; } @@ -553,7 +554,7 @@ void MemPeek(const char *token, unsigned long addr, unsigned long len, res->ok = 1; return; } - lstrcpynA(res->reason, "read failed", (int)sizeof(res->reason)); + McpStrCpyN(res->reason, "read failed", (int)sizeof(res->reason)); return; } @@ -568,13 +569,13 @@ void MemPeek(const char *token, unsigned long addr, unsigned long len, memset(&mbi, 0, sizeof(mbi)); if (VirtualQuery((LPCVOID)addr, &mbi, sizeof(mbi)) == 0) { - lstrcpynA(res->reason, "address not accessible", + McpStrCpyN(res->reason, "address not accessible", (int)sizeof(res->reason)); return; } n = MemRegionAccessibleDecision(&mbi, addr, len, 0); if (n == 0) { - lstrcpynA(res->reason, "address not accessible", + McpStrCpyN(res->reason, "address not accessible", (int)sizeof(res->reason)); return; } @@ -606,14 +607,14 @@ void MemPoke(const char *token, unsigned long addr, /* (1) Device write arm (/ALLOWMEMWRITE), the device half of the two-layer * arm. Clear -> refused with no write, binding every client. */ if (!AuditIsArmed()) { - lstrcpynA(res->reason, "memory writes not armed (/ALLOWMEMWRITE)", + McpStrCpyN(res->reason, "memory writes not armed (/ALLOWMEMWRITE)", (int)sizeof(res->reason)); return; } /* (2) Overflow-safe range floor. */ if (!MemRangeInBounds(addr, len, MEM_MAX_ACCESS)) { - lstrcpynA(res->reason, "range out of bounds", (int)sizeof(res->reason)); + McpStrCpyN(res->reason, "range out of bounds", (int)sizeof(res->reason)); return; } @@ -624,7 +625,7 @@ void MemPoke(const char *token, unsigned long addr, /* A device with no memory tier never writes (defence in depth: the bridge * prunes the tool, but a direct wire client must be refused too). */ if (tier == MEM_TIER_NONE) { - lstrcpynA(res->reason, "no memory capability", (int)sizeof(res->reason)); + McpStrCpyN(res->reason, "no memory capability", (int)sizeof(res->reason)); return; } @@ -635,17 +636,17 @@ void MemPoke(const char *token, unsigned long addr, * region is rejected WHOLE (never a partial pre-NT write). */ if (tier == MEM_TIER_PROCESS) { if (token == NULL || token[0] == '\0') { - lstrcpynA(res->reason, "token required on the process tier", + McpStrCpyN(res->reason, "token required on the process tier", (int)sizeof(res->reason)); return; } h = MemTokenHandle(token); if (h == NULL) { - lstrcpynA(res->reason, "invalid token", (int)sizeof(res->reason)); + McpStrCpyN(res->reason, "invalid token", (int)sizeof(res->reason)); return; } if (g_pWriteProcessMemory == NULL) { - lstrcpynA(res->reason, "WriteProcessMemory unavailable", + McpStrCpyN(res->reason, "WriteProcessMemory unavailable", (int)sizeof(res->reason)); return; } @@ -658,13 +659,13 @@ void MemPoke(const char *token, unsigned long addr, memset(&mbi, 0, sizeof(mbi)); if (VirtualQuery((LPCVOID)addr, &mbi, sizeof(mbi)) == 0) { - lstrcpynA(res->reason, "address not writable", + McpStrCpyN(res->reason, "address not writable", (int)sizeof(res->reason)); return; } n = MemRegionAccessibleDecision(&mbi, addr, len, 1); if (n < len) { - lstrcpynA(res->reason, "address not writable", + McpStrCpyN(res->reason, "address not writable", (int)sizeof(res->reason)); return; } @@ -673,7 +674,7 @@ void MemPoke(const char *token, unsigned long addr, /* (4) Audit sink writable (fail-closed): a poke that cannot be recorded * performs NO write. */ if (!AuditIsWritable()) { - lstrcpynA(res->reason, "audit sink not writable", + McpStrCpyN(res->reason, "audit sink not writable", (int)sizeof(res->reason)); return; } @@ -690,7 +691,7 @@ void MemPoke(const char *token, unsigned long addr, res->bytes_written = (unsigned long)wrote; res->partial = 1; } else { - lstrcpynA(res->reason, "write failed", (int)sizeof(res->reason)); + McpStrCpyN(res->reason, "write failed", (int)sizeof(res->reason)); return; } } else { @@ -716,7 +717,7 @@ void MemPoke(const char *token, unsigned long addr, * unlogged success. */ if (!AuditWritePoke(MemTierName(tier), auditTok, auditPid, auditCmd, addr, len, res->bytes_written, res->partial)) { - lstrcpynA(res->reason, + McpStrCpyN(res->reason, "audit write failed after the memory was modified - " "investigate the audit log", (int)sizeof(res->reason)); diff --git a/src/serial.c b/src/serial.c index fe9f395..b4ed13a 100644 --- a/src/serial.c +++ b/src/serial.c @@ -8,6 +8,7 @@ #include #include "serial.h" #include "uart.h" /* the Win32s tier gate + direct-UART route */ +#include "strutil.h" /* McpStrCpyN (the NT 3.1 floor lacks lstrcpynA) */ #ifdef TEST_BUILD /* The route SerialBackendOpen last selected (uart.h UartLastRouteForTest): the @@ -166,7 +167,7 @@ int SerialBackendOpen(const TransportConfig *cfg, Transport *out, * the on-target hardware acceptance. */ g_serial_route = UART_ROUTE_DIRECT_UART; if (err != NULL && errSize > 0) { - lstrcpynA(err, "win32s direct-uart route (test: no port I/O)", + McpStrCpyN(err, "win32s direct-uart route (test: no port I/O)", errSize); } return 0; @@ -181,7 +182,7 @@ int SerialBackendOpen(const TransportConfig *cfg, Transport *out, h = OpenSerialPort(cfg->port, cfg->baudRate); if (h == INVALID_HANDLE_VALUE) { if (err != NULL && errSize > 0) { - lstrcpynA(err, "failed to open serial port", errSize); + McpStrCpyN(err, "failed to open serial port", errSize); } return 0; } diff --git a/src/strutil.c b/src/strutil.c new file mode 100644 index 0000000..4d2e38e --- /dev/null +++ b/src/strutil.c @@ -0,0 +1,41 @@ +/* + * strutil.c - small string helpers the native-Win32 floor lacks. See strutil.h. + * + * Windows NT 3.1's kernel32 (the native-Win32 floor, July 1993) does not export + * lstrcpynA - it arrived in NT 3.51/Win95 - so the device carries its own. The + * copy is DBCS-aware (steps with CharNextA) so a truncated result never leaves a + * dangling double-byte lead, matching the real lstrcpynA. C89 / i386 / ANSI only. + * + * This is free and unencumbered software released into the public domain. + * See LICENSE for details (Unlicense). + */ +#include +#include "strutil.h" + +char *McpStrCpyN(char *dst, const char *src, int n) +{ + int i; + const char *p; + const char *q; + int clen; + + if (dst == NULL || n <= 0) { + return dst; + } + i = 0; + p = src; + while (p != NULL && *p != '\0') { + q = CharNextA(p); /* DBCS-aware single-character step */ + clen = (int)(q - p); + if (i + clen > n - 1) { + break; /* next char would not fit with the NUL */ + } + while (p < q) { + dst[i] = *p; + i++; + p++; + } + } + dst[i] = '\0'; + return dst; +} diff --git a/src/strutil.h b/src/strutil.h new file mode 100644 index 0000000..5aac7ee --- /dev/null +++ b/src/strutil.h @@ -0,0 +1,23 @@ +/* + * strutil.h - small string helpers the native-Win32 floor lacks. + * + * This is free and unencumbered software released into the public domain. + * See LICENSE for details (Unlicense). + */ +#ifndef MCP_STRUTIL_H +#define MCP_STRUTIL_H + +/* + * McpStrCpyN - bounded string copy with lstrcpynA semantics: copy at most + * (n-1) bytes from src to dst and always NUL-terminate when n > 0, never + * splitting a DBCS character. Returns dst. + * + * Supplied in the binary because Windows NT 3.1's kernel32 - the native-Win32 + * floor (1993) - does not export lstrcpynA (it arrived in NT 3.51/Win95). A + * static lstrcpynA import makes the device fail to load on NT 3.1 with + * ERROR_BAD_FORMAT/entry-point-not-found. (See plan/PHASE6.md in the host + * repo, NT 3.1 floor validation.) + */ +char *McpStrCpyN(char *dst, const char *src, int n); + +#endif /* MCP_STRUTIL_H */ diff --git a/src/tcp.c b/src/tcp.c index a6e267b..8d3a5f8 100644 --- a/src/tcp.c +++ b/src/tcp.c @@ -12,6 +12,7 @@ #include #include +#include "strutil.h" #include #include "tcp.h" @@ -206,7 +207,7 @@ int TcpBackendOpen(const TransportConfig *cfg, Transport *out, if (!TcpBackendProbe()) { if (err != NULL && errSize > 0) { - lstrcpynA(err, "winsock (wsock32.dll) not available", errSize); + McpStrCpyN(err, "winsock (wsock32.dll) not available", errSize); } return 0; } @@ -214,14 +215,14 @@ int TcpBackendOpen(const TransportConfig *cfg, Transport *out, if (!g_started) { if (g_ws.startup(MAKEWORD(1, 1), &wsa) != 0) { if (err != NULL && errSize > 0) { - lstrcpynA(err, "WSAStartup failed", errSize); + McpStrCpyN(err, "WSAStartup failed", errSize); } return 0; } if (LOBYTE(wsa.wVersion) != 1 || HIBYTE(wsa.wVersion) != 1) { g_ws.cleanup(); if (err != NULL && errSize > 0) { - lstrcpynA(err, "winsock 1.1 not supported", errSize); + McpStrCpyN(err, "winsock 1.1 not supported", errSize); } return 0; } @@ -231,7 +232,7 @@ int TcpBackendOpen(const TransportConfig *cfg, Transport *out, s = g_ws.socket(AF_INET, SOCK_STREAM, IPPROTO_TCP); if (s == INVALID_SOCKET) { if (err != NULL && errSize > 0) { - lstrcpynA(err, "socket() failed", errSize); + McpStrCpyN(err, "socket() failed", errSize); } return 0; } @@ -245,7 +246,7 @@ int TcpBackendOpen(const TransportConfig *cfg, Transport *out, SOCKET_ERROR) { g_ws.closesocket(s); if (err != NULL && errSize > 0) { - lstrcpynA(err, "bind() failed", errSize); + McpStrCpyN(err, "bind() failed", errSize); } return 0; } @@ -253,7 +254,7 @@ int TcpBackendOpen(const TransportConfig *cfg, Transport *out, if (g_ws.listen(s, 1) == SOCKET_ERROR) { g_ws.closesocket(s); if (err != NULL && errSize > 0) { - lstrcpynA(err, "listen() failed", errSize); + McpStrCpyN(err, "listen() failed", errSize); } return 0; } diff --git a/src/uart.c b/src/uart.c index ff83509..98dc65f 100644 --- a/src/uart.c +++ b/src/uart.c @@ -512,6 +512,7 @@ void UartDrainAndClose(const UartPortIo *io, UartDriver *drv) * probe, lives in serial.c where the dispatch branch is.) */ #include +#include "strutil.h" #include "transport.h" #include "feat.h" @@ -681,7 +682,7 @@ int UartBackendOpenDirect(const TransportConfig *cfg, Transport *out, base = uart_base_for_port(cfg->port); if (base == 0) { if (err != NULL && errSize > 0) { - lstrcpynA(err, "win32s direct-uart: unknown COM port", errSize); + McpStrCpyN(err, "win32s direct-uart: unknown COM port", errSize); } return 0; } @@ -691,7 +692,7 @@ int UartBackendOpenDirect(const TransportConfig *cfg, Transport *out, if (UartOpenSequence(&g_uart_io, &g_uart, base, divisor) != UART_OPEN_LIVE) { if (err != NULL && errSize > 0) { - lstrcpynA(err, + McpStrCpyN(err, "win32s direct-uart: no working UART at the COM port", errSize); } return 0; /* terminal - no degrade to the OS comm path (invariant 5) */ From c7c18d0f76642c1f5d4d9c52e19c6874a2aff43c Mon Sep 17 00:00:00 2001 From: David Connolly Date: Mon, 15 Jun 2026 22:00:54 +0100 Subject: [PATCH 2/7] Surface serial-open failures and add the \\.\COMn device-path fallback The NT 3.1 floor refuses CreateFileA on the bare "COMn" DOS-device alias where later Windows resolves it; retry once via the canonical "\\.\COMn" device-namespace form before failing (skipped if already prefixed). Also surface the real CreateFileA GetLastError() + the port tried in the error string, so an on-target open failure is diagnosable in a single boot (error 2 => not enumerated; 5 => in use; 87 => bad name form). No new import (wsprintfA is already user32); object stays opcode-clean. Diagnostic+fallback build for the parked #40 COM1-open blocker. Co-Authored-By: Claude Opus 4.8 --- src/serial.c | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/serial.c b/src/serial.c index b4ed13a..f488743 100644 --- a/src/serial.c +++ b/src/serial.c @@ -77,6 +77,22 @@ HANDLE OpenSerialPort(const char *portName, DWORD baudRate) GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, NULL); + /* + * NT 3.1 fallback: the bare "COMn" DOS-device alias can fail to resolve on + * the earliest NT loader where the canonical device-namespace form opens. + * Retry once via "\\.\COMn" before giving up; skipped when the caller + * already passed a "\\.\"-prefixed name. CreateFileA leaves GetLastError() + * from this final attempt intact for the caller to surface. + */ + if (hPort == INVALID_HANDLE_VALUE && + !(portName[0] == '\\' && portName[1] == '\\')) { + char devName[40]; + wsprintfA(devName, "\\\\.\\%s", portName); + hPort = CreateFileA(devName, + GENERIC_READ | GENERIC_WRITE, + 0, NULL, OPEN_EXISTING, 0, NULL); + } + if (hPort == INVALID_HANDLE_VALUE) { return INVALID_HANDLE_VALUE; } @@ -181,8 +197,17 @@ int SerialBackendOpen(const TransportConfig *cfg, Transport *out, h = OpenSerialPort(cfg->port, cfg->baudRate); if (h == INVALID_HANDLE_VALUE) { + DWORD gle = GetLastError(); if (err != NULL && errSize > 0) { - McpStrCpyN(err, "failed to open serial port", errSize); + char msg[128]; + /* Surface the real CreateFileA failure + the port tried, so an + * on-target open failure is diagnosable in one run: error 2 + * (ERROR_FILE_NOT_FOUND) => COM port not enumerated; 5 + * (ERROR_ACCESS_DENIED) => in use; 87 => bad param/name form. */ + wsprintfA(msg, + "failed to open serial port '%s' (CreateFileA error %lu)", + cfg->port, (unsigned long)gle); + McpStrCpyN(err, msg, errSize); } return 0; } From 05d265cf46c2cc4ff099a4412fddda2e49fff9c1 Mon Sep 17 00:00:00 2001 From: David Connolly Date: Mon, 15 Jun 2026 22:41:33 +0100 Subject: [PATCH 3/7] Open serial on NT 3.1 via GetCommState-first; report the failing stage Root-caused the NT 3.1 'failed to open serial port (error 87)': it was SetCommState, not CreateFile. NT's MODE opens COM1 and accepts 19200/n/8/1 fine (it does GetCommState -> modify -> SetCommState), but the device built its DCB from a memset(0) and called SetCommState cold -- a zeroed DCB has XonChar == XoffChar == 0, which the 1993 serial driver rejects with ERROR_INVALID_PARAMETER. Modern NT is lenient; NT 3.1 is not. Fix: seed the DCB from GetCommState, then change only baud/bytesize/parity/ stop + the flow-control flags (the robust idiom MODE/Terminal use). Also replace the mislabeled "CreateFileA error" string with the precise failing stage (CreateFileA/GetCommState/SetCommState/SetCommTimeouts) + error code, so one boot pinpoints the failure. \\.\COMn name fallback retained. No new import (GetCommState is kernel32); object stays opcode-clean. Pending on-target confirmation on the NT 3.1 guest. Co-Authored-By: Claude Opus 4.8 --- src/serial.c | 72 ++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 56 insertions(+), 16 deletions(-) diff --git a/src/serial.c b/src/serial.c index f488743..7909657 100644 --- a/src/serial.c +++ b/src/serial.c @@ -67,22 +67,37 @@ void BuildSerialTimeouts(COMMTIMEOUTS *timeouts) /* Write timeouts left at 0 (no timeout) */ } +/* + * The precise failure stage + error code from the last OpenSerialPort attempt, + * surfaced verbatim by SerialBackendOpen. Without it the caller cannot tell a + * CreateFile failure (port absent) from a SetCommState failure (DCB rejected): + * the NT 3.1 floor returns ERROR_INVALID_PARAMETER (87) from SetCommState on a + * from-zeroed DCB, which a generic "failed to open" message would hide. + */ +static char g_open_err[160]; + +static void open_err(const char *stage, const char *name, DWORD gle) +{ + wsprintfA(g_open_err, "%s('%s') failed, error %lu", + stage, name, (unsigned long)gle); +} + HANDLE OpenSerialPort(const char *portName, DWORD baudRate) { HANDLE hPort; DCB dcb; COMMTIMEOUTS timeouts; + g_open_err[0] = '\0'; + hPort = CreateFileA(portName, GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, NULL); /* - * NT 3.1 fallback: the bare "COMn" DOS-device alias can fail to resolve on - * the earliest NT loader where the canonical device-namespace form opens. - * Retry once via "\\.\COMn" before giving up; skipped when the caller - * already passed a "\\.\"-prefixed name. CreateFileA leaves GetLastError() - * from this final attempt intact for the caller to surface. + * NT 3.1 robustness: if the bare "COMn" DOS-device alias does not resolve, + * retry once via the canonical "\\.\COMn" device-namespace form (skipped + * when the caller already passed a "\\.\"-prefixed name). */ if (hPort == INVALID_HANDLE_VALUE && !(portName[0] == '\\' && portName[1] == '\\')) { @@ -94,17 +109,46 @@ HANDLE OpenSerialPort(const char *portName, DWORD baudRate) } if (hPort == INVALID_HANDLE_VALUE) { + open_err("CreateFileA", portName, GetLastError()); return INVALID_HANDLE_VALUE; } - BuildSerialDCB(baudRate, &dcb); + /* + * GetCommState-first: seed the DCB from the driver's current (valid) state, + * then change only the fields we need. A from-zeroed DCB leaves + * XonChar == XoffChar == 0 and other fields NT 3.1's serial driver rejects + * with ERROR_INVALID_PARAMETER (87); get-then-modify preserves the driver's + * valid defaults. This is what MODE/Terminal do, and they open COM1 on + * NT 3.1 where the cold from-zero SetCommState failed. + */ + memset(&dcb, 0, sizeof(DCB)); + dcb.DCBlength = sizeof(DCB); + if (!GetCommState(hPort, &dcb)) { + open_err("GetCommState", portName, GetLastError()); + CloseHandle(hPort); + return INVALID_HANDLE_VALUE; + } + dcb.BaudRate = baudRate; + dcb.ByteSize = 8; + dcb.Parity = NOPARITY; + dcb.StopBits = ONESTOPBIT; + dcb.fBinary = TRUE; + dcb.fParity = FALSE; + dcb.fOutxCtsFlow = FALSE; + dcb.fOutxDsrFlow = FALSE; + dcb.fDtrControl = DTR_CONTROL_ENABLE; + dcb.fRtsControl = RTS_CONTROL_ENABLE; + dcb.fOutX = FALSE; + dcb.fInX = FALSE; if (!SetCommState(hPort, &dcb)) { + open_err("SetCommState", portName, GetLastError()); CloseHandle(hPort); return INVALID_HANDLE_VALUE; } BuildSerialTimeouts(&timeouts); if (!SetCommTimeouts(hPort, &timeouts)) { + open_err("SetCommTimeouts", portName, GetLastError()); CloseHandle(hPort); return INVALID_HANDLE_VALUE; } @@ -197,17 +241,13 @@ int SerialBackendOpen(const TransportConfig *cfg, Transport *out, h = OpenSerialPort(cfg->port, cfg->baudRate); if (h == INVALID_HANDLE_VALUE) { - DWORD gle = GetLastError(); if (err != NULL && errSize > 0) { - char msg[128]; - /* Surface the real CreateFileA failure + the port tried, so an - * on-target open failure is diagnosable in one run: error 2 - * (ERROR_FILE_NOT_FOUND) => COM port not enumerated; 5 - * (ERROR_ACCESS_DENIED) => in use; 87 => bad param/name form. */ - wsprintfA(msg, - "failed to open serial port '%s' (CreateFileA error %lu)", - cfg->port, (unsigned long)gle); - McpStrCpyN(err, msg, errSize); + /* g_open_err names the precise failing stage + error code, set by + * OpenSerialPort: CreateFileA / GetCommState / SetCommState / + * SetCommTimeouts. One boot pinpoints the NT 3.1 floor failure. */ + McpStrCpyN(err, + g_open_err[0] ? g_open_err : "failed to open serial port", + errSize); } return 0; } From 79e7240d21c09e9d53fbb941ad5030b9fd28a1d3 Mon Sep 17 00:00:00 2001 From: David Connolly Date: Mon, 15 Jun 2026 23:38:04 +0100 Subject: [PATCH 4/7] Fill the propagate-found test gaps: McpStrCpyN and the handle-inherit fallback McpStrCpyN (the NT 3.1 bounded copy, used at ~50 sites) had no direct test. Add a host theft PBT (tests/host/theft_strutil.c, 5 properties x 50k, ASan): NUL-terminated & length <= n-1; result is a byte-prefix of src; no DBCS character split (re-walk with a cp932-style CharNextA shim lands exactly on the terminator); bounded write (dst sized exactly n, ASan red-zones catch overflow); edges (dst==NULL, n<=0). Mirror on-target via prop.h (tests/test_strutil.c). F2's ClearHandleInherit DuplicateHandle fallback (taken only when NT 3.1 lacks SetHandleInformation) was untested. Expose a TEST_BUILD hook and add test_exec_ops cases: force the probe NULL, confirm GetHandleInformation shows HANDLE_FLAG_INHERIT cleared on a still-valid handle; the API path; the NULL/invalid guards. Also fix a pre-existing host-pbt breakage: the theft_catalog build line lacked src/strutil.c, so catalog.c's McpStrCpyN calls were undefined refs (broken since the F3 swap) -- host-pbt was failing in CI on this branch. Co-Authored-By: Claude Opus 4.8 --- CMakeLists.txt | 12 +- build.sh | 7 +- src/exec_ops.c | 10 ++ src/exec_ops.h | 10 ++ tests/host/theft_strutil.c | 316 +++++++++++++++++++++++++++++++++++++ tests/host/win32_shim.h | 46 +++++- tests/test_exec_ops.c | 123 +++++++++++++++ tests/test_strutil.c | 144 +++++++++++++++++ 8 files changed, 662 insertions(+), 6 deletions(-) create mode 100644 tests/host/theft_strutil.c create mode 100644 tests/test_strutil.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 9a8f493..c1e2add 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -176,6 +176,13 @@ target_link_libraries(test_file_ops PRIVATE kernel32 user32) add_executable(test_pbt_base64 tests/test_pbt_base64.c src/base64.c) target_link_libraries(test_pbt_base64 PRIVATE kernel32) +# test_strutil: the prop.h on-target mirror of the McpStrCpyN properties +# (bounded/NUL/prefix/no-split), proving the device's own DBCS-aware bounded +# copy holds on the shipped C89/i386 path. The deep 50k-trial theft host run +# lives in tests/host/theft_strutil.c (built by build.sh host-pbt). +add_executable(test_strutil tests/test_strutil.c src/strutil.c) +target_link_libraries(test_strutil PRIVATE kernel32 user32) + # --- Phase 4 test targets --------------------------------------------------- add_executable(test_feat tests/test_feat.c src/feat.c) target_link_libraries(test_feat PRIVATE kernel32) @@ -200,6 +207,9 @@ target_link_libraries(test_catalog PRIVATE kernel32) add_executable(test_exec_ops tests/test_exec_ops.c src/exec_ops.c src/feat.c src/binfmt.c src/encoding.c src/charset_tables.c src/charset_tables_data.c src/strutil.c) +# TEST_BUILD exposes ExecClearHandleInheritForTest (the static +# ClearHandleInherit hook) so the NT 3.1 DuplicateHandle fallback is testable. +target_compile_definitions(test_exec_ops PRIVATE TEST_BUILD) target_link_libraries(test_exec_ops PRIVATE kernel32) add_executable(test_pty_exec tests/test_pty_exec.c src/pty_exec.c src/feat.c) @@ -277,7 +287,7 @@ add_custom_command(TARGET test_catalog POST_BUILD # CMAKE_CROSSCOMPILING_EMULATOR (set by the toolchain: empty for native # WSL-interop execution, "wine" otherwise) is applied to each test command. foreach(t test_json test_transport test_tcp test_serial - test_base64 test_file_ops test_pbt_base64 + test_base64 test_file_ops test_pbt_base64 test_strutil test_feat test_argv test_binfmt test_catalog test_exec_ops test_pty_exec test_toolchain_probe test_mem_ops test_audit test_encoding test_pbt_encoding test_uart) diff --git a/build.sh b/build.sh index 347a0af..291950b 100755 --- a/build.sh +++ b/build.sh @@ -27,7 +27,11 @@ if [ "$1" = "host-pbt" ]; then gcc $HFLAGS tests/host/theft_base64.c src/base64.c build/host/theft/*.o -lm -o build/host/theft_base64 gcc $HFLAGS tests/host/theft_json.c src/json_parser.c build/host/theft/*.o -lm -o build/host/theft_json gcc $HFLAGS tests/host/theft_argv.c src/argv.c build/host/theft/*.o -lm -o build/host/theft_argv - gcc $HFLAGS tests/host/theft_catalog.c src/catalog.c src/json_parser.c build/host/theft/*.o -lm -o build/host/theft_catalog + gcc $HFLAGS tests/host/theft_catalog.c src/catalog.c src/json_parser.c src/strutil.c build/host/theft/*.o -lm -o build/host/theft_catalog + # strutil: the DBCS-aware bounded copy (McpStrCpyN). CharNextA resolves to + # the cp932-style shim (tests/host/win32_shim.h) so the no-split property + # is deterministically testable - the bounded/NUL/prefix/no-split pins at 50k. + gcc $HFLAGS tests/host/theft_strutil.c src/strutil.c build/host/theft/*.o -lm -o build/host/theft_strutil # mem_ops: only the two pure arithmetic guards compile natively # (MEM_OPS_HOST_PURE excludes the Win32 surface) - the off-by-overflow pin. gcc $HFLAGS -DMEM_OPS_HOST_PURE tests/host/theft_mem.c src/mem_ops.c build/host/theft/*.o -lm -o build/host/theft_mem @@ -45,6 +49,7 @@ if [ "$1" = "host-pbt" ]; then build/host/theft_json build/host/theft_argv build/host/theft_catalog + build/host/theft_strutil build/host/theft_mem build/host/theft_encoding build/host/theft_uart diff --git a/src/exec_ops.c b/src/exec_ops.c index ea5ee6d..087b17f 100644 --- a/src/exec_ops.c +++ b/src/exec_ops.c @@ -155,6 +155,16 @@ static void ClearHandleInherit(HANDLE *ph) } } +#ifdef TEST_BUILD +/* Test-only hook: invoke the static ClearHandleInherit so test_exec_ops can + * pin both the SetHandleInformation route and the NT 3.1 DuplicateHandle + * fallback (forced by NULLing g_features.pSetHandleInformation). */ +void ExecClearHandleInheritForTest(HANDLE *ph) +{ + ClearHandleInherit(ph); +} +#endif + /* * PumpPipe - polling-path non-blocking drain (Q3). PeekNamedPipe to find * how many bytes are available; ReadFile up to the smaller of that and diff --git a/src/exec_ops.h b/src/exec_ops.h index 1c452eb..7c0a9bb 100644 --- a/src/exec_ops.h +++ b/src/exec_ops.h @@ -110,4 +110,14 @@ int ExecOpRun( char *errMsg, int errSize ); +#ifdef TEST_BUILD +/* + * ExecClearHandleInheritForTest - test-only hook onto the static + * ClearHandleInherit, so test_exec_ops can pin BOTH routes: the + * SetHandleInformation path and the NT 3.1 DuplicateHandle fallback (reached + * by forcing g_features.pSetHandleInformation to NULL in the test). + */ +void ExecClearHandleInheritForTest(HANDLE *ph); +#endif + #endif /* EXEC_OPS_H */ diff --git a/tests/host/theft_strutil.c b/tests/host/theft_strutil.c new file mode 100644 index 0000000..e94c0a2 --- /dev/null +++ b/tests/host/theft_strutil.c @@ -0,0 +1,316 @@ +/* + * theft_strutil.c - host-native property-based tests for src/strutil.c + * (McpStrCpyN, the device's own DBCS-aware bounded copy - the NT 3.1 floor + * lacks lstrcpynA). + * + * theft host PBT harness (CLAUDE.md "two frameworks"). The shim's CharNextA + * (tests/host/win32_shim.h) models a cp932-style DBCS codepage so the + * no-split property is DETERMINISTICALLY testable on Linux. + * + * Properties (>= 50000 trials each, autoshrinking) over random byte strings + * + random n (including n <= 0 and n == 1): + * P1 terminated n > 0 => result is NUL-terminated and strlen(result) <= n-1. + * P2 prefix result bytes are a byte-exact prefix of src. + * P3 no_split re-walking the result with CharNextA from the start lands + * EXACTLY on the terminator (the last copied char is whole; + * no dangling DBCS lead byte). + * P4 bounded dst is allocated as EXACTLY n bytes inside an ASan-guarded + * region; McpStrCpyN never writes at or beyond dst[n]. + * P5 edges dst == NULL returns NULL and does not crash; n <= 0 writes + * nothing (a poisoned guard byte before the buffer survives). + * + * Module under test stays C89; this harness is C99 + POSIX, built natively + * with gcc + ASan/UBSan on Linux. The Win32 surface (CharNextA) resolves to + * tests/host/win32_shim.h through tests/host/windows.h. + * + * This is free and unencumbered software released into the public domain. + */ + +#include +#include +#include /* shim: CharNextA */ +#include "theft.h" +#include "strutil.h" + +#define TRIALS 50000 +#define MAX_SRC 256 +#define SEED 0x57271234CAFEULL + +/* A generated random NUL-terminated src string plus a bound n. The src is a + * mix of single bytes and well-formed cp932-style lead/trail pairs so the + * no-split property has real double-byte characters to (not) split. n ranges + * into <= 0 and small values. */ +struct input { + char src[MAX_SRC + 1]; /* always NUL-terminated */ + int src_len; /* strlen(src) */ + int n; /* bound passed to McpStrCpyN, may be <= 0 */ +}; + +static int is_lead(unsigned char c) +{ + return (c >= 0x81 && c <= 0x9F) || (c >= 0xE0 && c <= 0xFC); +} + +static enum theft_alloc_res +input_alloc(struct theft *t, void *env, void **out) +{ + struct input *in; + int target, i; + (void)env; + + in = malloc(sizeof(*in)); + if (in == NULL) { + return THEFT_ALLOC_ERROR; + } + /* 8 bits -> 0..255 masked into 0..MAX_SRC; small pools -> short/empty. */ + target = (int)(theft_random_bits(t, 8) % (MAX_SRC + 1)); + i = 0; + while (i < target) { + unsigned char b = (unsigned char)theft_random_bits(t, 8); + if (b == 0) { + /* Never embed an interior NUL: it would end the string early and + * the generator's intended length would not be realised. Bias to + * a printable byte instead. */ + b = (unsigned char)('A' + (theft_random_bits(t, 5) % 26)); + } + if (is_lead(b) && i + 1 < MAX_SRC && (i + 1) < target) { + /* Emit a well-formed double-byte char: lead + non-NUL trail. */ + unsigned char trail = (unsigned char)(theft_random_bits(t, 8)); + if (trail == 0) { + trail = 0x40; /* a valid-ish cp932 trail */ + } + in->src[i++] = (char)b; + in->src[i++] = (char)trail; + } else if (is_lead(b)) { + /* No room for a trail: substitute a single ASCII byte so we never + * leave a dangling lead in the GENERATED src itself. */ + in->src[i++] = (char)('a' + (b % 26)); + } else { + in->src[i++] = (char)b; + } + } + in->src[i] = '\0'; + in->src_len = i; + /* n in -2 .. src_len+2, so n <= 0, n == 1, exact-fit and over-fit all + * occur. 6 bits -> 0..63 then shifted to include the negatives. */ + in->n = (int)(theft_random_bits(t, 6) % (unsigned)(in->src_len + 5)) - 2; + *out = in; + return THEFT_ALLOC_OK; +} + +static struct theft_type_info input_info = { + .alloc = input_alloc, + .free = theft_generic_free_cb, + .autoshrink_config = { .enable = true }, +}; + +/* P1: n > 0 => NUL-terminated, strlen(result) <= n-1. */ +static enum theft_trial_res +prop_terminated(struct theft *t, void *arg1) +{ + struct input *in = (struct input *)arg1; + char *dst; + int n; + (void)t; + + n = in->n; + if (n <= 0) { + return THEFT_TRIAL_PASS; /* covered by P5 */ + } + dst = malloc((size_t)n); + if (dst == NULL) { + return THEFT_TRIAL_ERROR; + } + memset(dst, 'Z', (size_t)n); + McpStrCpyN(dst, in->src, n); + if ((int)strlen(dst) > n - 1) { + free(dst); + return THEFT_TRIAL_FAIL; + } + /* NUL-terminated: strlen stayed inside the buffer, so dst[strlen] is the + * NUL within [0, n-1]. */ + free(dst); + return THEFT_TRIAL_PASS; +} + +/* P2: result bytes are a byte-exact prefix of src. */ +static enum theft_trial_res +prop_prefix(struct theft *t, void *arg1) +{ + struct input *in = (struct input *)arg1; + char *dst; + int n, len; + (void)t; + + n = in->n; + if (n <= 0) { + return THEFT_TRIAL_PASS; + } + dst = malloc((size_t)n); + if (dst == NULL) { + return THEFT_TRIAL_ERROR; + } + McpStrCpyN(dst, in->src, n); + len = (int)strlen(dst); + if (len > in->src_len) { + free(dst); + return THEFT_TRIAL_FAIL; /* longer than the source: impossible */ + } + if (memcmp(dst, in->src, (size_t)len) != 0) { + free(dst); + return THEFT_TRIAL_FAIL; + } + free(dst); + return THEFT_TRIAL_PASS; +} + +/* P3: re-walking the result with CharNextA lands EXACTLY on the terminator. */ +static enum theft_trial_res +prop_no_split(struct theft *t, void *arg1) +{ + struct input *in = (struct input *)arg1; + char *dst; + const char *p; + int n; + (void)t; + + n = in->n; + if (n <= 0) { + return THEFT_TRIAL_PASS; + } + dst = malloc((size_t)n); + if (dst == NULL) { + return THEFT_TRIAL_ERROR; + } + McpStrCpyN(dst, in->src, n); + /* Walk character-by-character; we must arrive at the NUL exactly, never + * step past it (which CharNextA refuses) and never stop short with a + * dangling lead byte. */ + p = dst; + while (*p != '\0') { + const char *q = CharNextA(p); + if (q == p) { + free(dst); + return THEFT_TRIAL_FAIL; /* no progress: malformed */ + } + p = q; + } + /* p now points at the terminator. A split would have left a trailing lead + * byte that CharNextA treats as a single byte stepping onto the NUL, but + * McpStrCpyN must never copy such a fragment: assert the last character + * boundary in dst matches a boundary in src too. Re-walk src to the same + * byte offset and confirm it is a character boundary there. */ + { + int off = (int)(p - dst); + const char *s = in->src; + while ((int)(s - in->src) < off && *s != '\0') { + s = CharNextA(s); + } + if ((int)(s - in->src) != off) { + free(dst); + return THEFT_TRIAL_FAIL; /* result end is mid-character in src */ + } + } + free(dst); + return THEFT_TRIAL_PASS; +} + +/* P4: bounded write - dst is EXACTLY n bytes; ASan red-zones catch any write + * at or beyond dst[n]. (The malloc'd block is exactly n; a stray write trips + * ASan and aborts the run.) */ +static enum theft_trial_res +prop_bounded(struct theft *t, void *arg1) +{ + struct input *in = (struct input *)arg1; + char *dst; + int n; + (void)t; + + n = in->n; + if (n <= 0) { + return THEFT_TRIAL_PASS; + } + dst = malloc((size_t)n); /* EXACTLY n bytes, ASan-guarded */ + if (dst == NULL) { + return THEFT_TRIAL_ERROR; + } + McpStrCpyN(dst, in->src, n); + /* If McpStrCpyN wrote at/beyond dst[n] ASan already aborted. Belt: the + * terminator sits within the buffer. */ + if (dst[strlen(dst)] != '\0') { + free(dst); + return THEFT_TRIAL_FAIL; + } + free(dst); + return THEFT_TRIAL_PASS; +} + +/* P5: edges - dst == NULL returns NULL; n <= 0 writes nothing. */ +static enum theft_trial_res +prop_edges(struct theft *t, void *arg1) +{ + struct input *in = (struct input *)arg1; + char *guarded; + char *dst; + (void)t; + + /* dst == NULL: returns NULL, no crash, for any n. */ + if (McpStrCpyN(NULL, in->src, in->n) != NULL) { + return THEFT_TRIAL_FAIL; + } + if (McpStrCpyN(NULL, in->src, 16) != NULL) { + return THEFT_TRIAL_FAIL; + } + + /* n <= 0 writes nothing: give a 1-byte buffer with a known sentinel and + * confirm McpStrCpyN leaves it untouched (no NUL written, no copy). */ + guarded = malloc(1); + if (guarded == NULL) { + return THEFT_TRIAL_ERROR; + } + guarded[0] = (char)0xAB; + dst = McpStrCpyN(guarded, in->src, 0); + if (dst != guarded) { + free(guarded); + return THEFT_TRIAL_FAIL; /* must return dst unchanged */ + } + if ((unsigned char)guarded[0] != 0xAB) { + free(guarded); + return THEFT_TRIAL_FAIL; /* wrote into a n<=0 buffer */ + } + dst = McpStrCpyN(guarded, in->src, -5); + if (dst != guarded || (unsigned char)guarded[0] != 0xAB) { + free(guarded); + return THEFT_TRIAL_FAIL; + } + free(guarded); + return THEFT_TRIAL_PASS; +} + +static int run(const char *name, theft_propfun1 *prop) +{ + struct theft_run_config cfg = { + .name = name, + .prop1 = prop, + .type_info = { &input_info }, + .trials = TRIALS, + .seed = SEED, + }; + enum theft_run_res res = theft_run(&cfg); + printf(" %-28s %s (%d trials)\n", name, + res == THEFT_RUN_PASS ? "PASS" : "FAIL", TRIALS); + return res == THEFT_RUN_PASS ? 0 : 1; +} + +int main(void) +{ + int fails = 0; + printf("theft_strutil (src/strutil.c):\n"); + fails += run("strutil/terminated", prop_terminated); + fails += run("strutil/prefix", prop_prefix); + fails += run("strutil/no_split", prop_no_split); + fails += run("strutil/bounded", prop_bounded); + fails += run("strutil/edges", prop_edges); + printf("%s\n", fails == 0 ? "ALL PASS" : "FAILURES"); + return fails == 0 ? 0 : 1; +} diff --git a/tests/host/win32_shim.h b/tests/host/win32_shim.h index 77b5aee..c6c9af6 100644 --- a/tests/host/win32_shim.h +++ b/tests/host/win32_shim.h @@ -32,6 +32,16 @@ #include #include /* strcasecmp */ +/* Each host test TU touches only the subset of this shim it needs (catalog + * uses the file I/O + ANSI strings; strutil uses only CharNextA), so under + * -Werror an unused static here is an error. Mark the static helpers + * possibly-unused; this is a gcc-only host harness (CLAUDE.md "two frameworks"). */ +#if defined(__GNUC__) +#define SHIM_UNUSED __attribute__((__unused__)) +#else +#define SHIM_UNUSED +#endif + /* ---- Win32 scalar types (host approximations) ---- */ typedef void * HANDLE; typedef unsigned long DWORD; @@ -63,7 +73,7 @@ typedef void * LPSECURITY_ATTRIBUTES; * CreateFileA - open PATH read-only. Only the read-only path catalog.c uses * is honoured; the access/share/disposition flags are ignored. */ -static HANDLE CreateFileA(LPCSTR path, DWORD access, DWORD share, +static SHIM_UNUSED HANDLE CreateFileA(LPCSTR path, DWORD access, DWORD share, LPSECURITY_ATTRIBUTES sa, DWORD disp, DWORD attrs, HANDLE tmpl) { @@ -81,7 +91,7 @@ static HANDLE CreateFileA(LPCSTR path, DWORD access, DWORD share, * Returns FALSE only on a hard read error (matches catalog.c's expectations: * EOF is read==0 with a TRUE return). */ -static BOOL ReadFile(HANDLE h, void *buf, DWORD toRead, LPDWORD read, +static SHIM_UNUSED BOOL ReadFile(HANDLE h, void *buf, DWORD toRead, LPDWORD read, LPVOID overlapped) { size_t n; @@ -96,7 +106,7 @@ static BOOL ReadFile(HANDLE h, void *buf, DWORD toRead, LPDWORD read, return TRUE; } -static BOOL CloseHandle(HANDLE h) +static SHIM_UNUSED BOOL CloseHandle(HANDLE h) { if (h == NULL || h == INVALID_HANDLE_VALUE) { return FALSE; @@ -109,7 +119,7 @@ static BOOL CloseHandle(HANDLE h) /* lstrcpynA copies at most count-1 chars and always NUL-terminates (Win32 * semantics), returning the destination. */ -static char *lstrcpynA(char *dst, const char *src, int count) +static SHIM_UNUSED char *lstrcpynA(char *dst, const char *src, int count) { int i; if (count <= 0) { @@ -126,4 +136,32 @@ static char *lstrcpynA(char *dst, const char *src, int count) #define lstrcmpiA(a, b) strcasecmp((a), (b)) #define lstrlenA(s) ((int)strlen(s)) +/* + * CharNextA - DBCS-aware single-character step (Win32 LPSTR CharNextA(LPCSTR)). + * src/strutil.c steps with this to never split a multibyte character. + * + * The host harness is native Linux with no real CharNextA. To make the + * no-split property DETERMINISTICALLY testable, this shim models a cp932-style + * DBCS codepage: a lead byte in 0x81-0x9F or 0xE0-0xFC followed by a non-NUL + * trail byte advances by 2 bytes (one double-byte character); otherwise it + * advances by 1. At the terminating NUL it does not advance past it. This is + * the same lead-byte test the real CharNextA applies under a DBCS ACP. + */ +static SHIM_UNUSED char *CharNextA(const char *p) +{ + unsigned char lead; + if (p == NULL) { + return (char *)p; + } + if (*p == '\0') { + return (char *)p; /* never step past the terminator */ + } + lead = (unsigned char)*p; + if (((lead >= 0x81 && lead <= 0x9F) || (lead >= 0xE0 && lead <= 0xFC)) && + p[1] != '\0') { + return (char *)(p + 2); /* whole double-byte character */ + } + return (char *)(p + 1); +} + #endif /* WIN32_SHIM_H */ diff --git a/tests/test_exec_ops.c b/tests/test_exec_ops.c index 4bc297b..480ffc2 100644 --- a/tests/test_exec_ops.c +++ b/tests/test_exec_ops.c @@ -684,6 +684,124 @@ TEST_CASE(killed_by_vocabulary) TEST_ASSERT_INT_EQUAL(4, EXEC_KILLED_CPU_CAP, "cpu_cap = 4"); } +/* ================================================================ + * ClearHandleInherit - both routes. Obligation (propagate gap, F2): the + * static ClearHandleInherit clears HANDLE_FLAG_INHERIT via + * SetHandleInformation when present, ELSE (the NT 3.1 floor) via a + * non-inheritable DuplicateHandle + CloseHandle(original) + *ph = dup. The + * fallback route had no test. ExecClearHandleInheritForTest (TEST_BUILD) + * reaches the static helper; NULLing g_features.pSetHandleInformation forces + * the fallback on a host whose kernel32 DOES export SetHandleInformation + * (mingw/wine/native), simulating the NT 3.1 path. + * + * GetHandleInformation/SetHandleInformation exist on the host even though NT + * 3.1 lacks them - the test forces the probe NULL only inside exec_ops, and + * still reads back the flag through the host's real GetHandleInformation. + * ================================================================ */ + +/* Same type as g_features.pSetHandleInformation, so save/restore stays a + * function-pointer assignment (C89 forbids fnptr<->void* casts). */ +typedef BOOL (WINAPI *SetHandleInfoFn)(HANDLE, DWORD, DWORD); + +/* Make an inheritable pipe-read handle (HANDLE_FLAG_INHERIT set). */ +static HANDLE make_inheritable_handle(void) +{ + SECURITY_ATTRIBUTES sa; + HANDLE rd, wr; + sa.nLength = sizeof(sa); + sa.lpSecurityDescriptor = NULL; + sa.bInheritHandle = TRUE; + if (!CreatePipe(&rd, &wr, &sa, 0)) { + return NULL; + } + CloseHandle(wr); /* we only need the read end */ + return rd; /* inheritable per the SECURITY_ATTRIBUTES */ +} + +/* ClearHandleInherit, fallback (NT 3.1 DuplicateHandle) route. */ +TEST_CASE(clear_inherit_fallback) +{ + SetHandleInfoFn saved; + HANDLE h; + DWORD flags; + saved = g_features.pSetHandleInformation; + + h = make_inheritable_handle(); + TEST_ASSERT(h != NULL && h != INVALID_HANDLE_VALUE, "made inheritable handle"); + flags = 0; + TEST_ASSERT(GetHandleInformation(h, &flags) != 0, "read initial flags"); + TEST_ASSERT((flags & HANDLE_FLAG_INHERIT) != 0, "handle starts inheritable"); + + /* Force the NT 3.1 fallback path. */ + g_features.pSetHandleInformation = NULL; + ExecClearHandleInheritForTest(&h); + g_features.pSetHandleInformation = saved; /* restore */ + + TEST_ASSERT(h != NULL && h != INVALID_HANDLE_VALUE, + "fallback left a valid handle"); + flags = 0; + TEST_ASSERT(GetHandleInformation(h, &flags) != 0, + "duplicated handle is queryable"); + TEST_ASSERT((flags & HANDLE_FLAG_INHERIT) == 0, + "fallback cleared HANDLE_FLAG_INHERIT"); + CloseHandle(h); +} + +/* ClearHandleInherit, API (SetHandleInformation) route. */ +TEST_CASE(clear_inherit_api) +{ + HANDLE h; + DWORD flags; + + if (g_features.pSetHandleInformation == NULL) { + TEST_ASSERT(1, "skipped: host has no SetHandleInformation probe"); + return; + } + h = make_inheritable_handle(); + TEST_ASSERT(h != NULL && h != INVALID_HANDLE_VALUE, "made inheritable handle"); + flags = 0; + TEST_ASSERT(GetHandleInformation(h, &flags) != 0, "read initial flags"); + TEST_ASSERT((flags & HANDLE_FLAG_INHERIT) != 0, "handle starts inheritable"); + + ExecClearHandleInheritForTest(&h); /* probe present -> API route */ + + TEST_ASSERT(h != NULL && h != INVALID_HANDLE_VALUE, "handle still valid"); + flags = 0; + TEST_ASSERT(GetHandleInformation(h, &flags) != 0, "handle queryable"); + TEST_ASSERT((flags & HANDLE_FLAG_INHERIT) == 0, + "API route cleared HANDLE_FLAG_INHERIT"); + CloseHandle(h); +} + +/* ClearHandleInherit guards: NULL ph, NULL handle, INVALID_HANDLE_VALUE all + * no-op without a crash, on both routes. */ +TEST_CASE(clear_inherit_guards) +{ + SetHandleInfoFn saved; + HANDLE h; + int route; + + saved = g_features.pSetHandleInformation; + for (route = 0; route < 2; route++) { + /* route 0: API present (if any); route 1: forced fallback. */ + if (route == 1) { + g_features.pSetHandleInformation = NULL; + } + /* NULL ph: no crash, returns. */ + ExecClearHandleInheritForTest(NULL); + /* NULL handle: no crash. */ + h = NULL; + ExecClearHandleInheritForTest(&h); + TEST_ASSERT(h == NULL, "NULL handle left unchanged"); + /* INVALID_HANDLE_VALUE: no crash. */ + h = INVALID_HANDLE_VALUE; + ExecClearHandleInheritForTest(&h); + TEST_ASSERT(h == INVALID_HANDLE_VALUE, "INVALID handle left unchanged"); + } + g_features.pSetHandleInformation = saved; /* restore */ + TEST_ASSERT(1, "guards survived both routes"); +} + int main(void) { /* Unbuffered: a hang mid-suite must not swallow progress output. */ @@ -723,6 +841,11 @@ int main(void) RUN_TEST(pe32_timeout_not_still_active); RUN_TEST(killed_by_vocabulary); + /* F2: ClearHandleInherit - both routes + guards (propagate gap). */ + RUN_TEST(clear_inherit_fallback); + RUN_TEST(clear_inherit_api); + RUN_TEST(clear_inherit_guards); + print_test_summary(); return g_tests_failed; } diff --git a/tests/test_strutil.c b/tests/test_strutil.c new file mode 100644 index 0000000..a9dd780 --- /dev/null +++ b/tests/test_strutil.c @@ -0,0 +1,144 @@ +/* + * test_strutil.c - prop.h on-target mirror of the McpStrCpyN properties + * (src/strutil.c, the device's own DBCS-aware bounded copy - the NT 3.1 + * floor lacks lstrcpynA). + * + * Mirrors the theft host properties (tests/host/theft_strutil.c) at lower + * trial counts so the bounded/NUL/prefix/no-split contract is proven on the + * actual shipped C89/i386 build, not only natively. Here CharNextA is the + * REAL Win32 one - single-byte on the build codepage - so the no-split + * property holds trivially on this host; the test still pins the C89 build's + * bounded-write, always-NUL-terminated and byte-exact-prefix behaviour and + * the n <= 0 / dst == NULL guards. + * + * Uses prop.h (minimal C89 PBT framework, fixed seeds, lower trial counts). + * + * This is free and unencumbered software released into the public domain. + * See LICENSE for details (Unlicense). + */ + +#define PROP_IMPLEMENTATION +#include "prop.h" +#include "strutil.h" +#include /* CharNextA */ +#include +#include + +#define MAX_SRC 128 + +/* Fill buf with a random NUL-terminated src of length 0..MAX_SRC-1, using + * non-NUL bytes so the intended length is realised. Returns the length. */ +static int gen_src(prop_ctx *_pc, char *buf) +{ + int len, i; + len = PROP_INT(0, MAX_SRC - 1); + for (i = 0; i < len; i++) { + int b; + b = PROP_INT(1, 255); /* never an interior NUL */ + buf[i] = (char)b; + } + buf[len] = '\0'; + return len; +} + +/* P1: n > 0 => result is NUL-terminated and strlen(result) <= n-1. */ +PROP_TEST(strutil_terminated) { + char src[MAX_SRC + 1]; + char dst[MAX_SRC + 1]; + int n, rlen; + + gen_src(_pc, src); + n = PROP_INT(1, MAX_SRC + 1); + memset(dst, 'Z', sizeof(dst)); + McpStrCpyN(dst, src, n); + rlen = (int)strlen(dst); /* terminates: strlen stops at the NUL */ + PROP_CHECK(rlen <= n - 1); +} + +/* P2: result bytes are a byte-exact prefix of src. */ +PROP_TEST(strutil_prefix) { + char src[MAX_SRC + 1]; + char dst[MAX_SRC + 1]; + int n, rlen, srclen; + + srclen = gen_src(_pc, src); + n = PROP_INT(1, MAX_SRC + 1); + McpStrCpyN(dst, src, n); + rlen = (int)strlen(dst); + PROP_CHECK(rlen <= srclen); + PROP_CHECK(memcmp(dst, src, (size_t)rlen) == 0); +} + +/* P3: re-walking the result with CharNextA lands EXACTLY on the terminator + * (never splits a character; trivially holds on a single-byte build cp). */ +PROP_TEST(strutil_no_split) { + char src[MAX_SRC + 1]; + char dst[MAX_SRC + 1]; + const char *p; + int n; + + gen_src(_pc, src); + n = PROP_INT(1, MAX_SRC + 1); + McpStrCpyN(dst, src, n); + p = dst; + while (*p != '\0') { + const char *q; + q = CharNextA(p); + PROP_CHECK(q != p); /* always makes progress */ + p = q; + } + /* arrived exactly at the terminator */ + PROP_CHECK(*p == '\0'); +} + +/* P4: bounded write - allocate dst as EXACTLY n bytes with a poisoned guard + * byte immediately after, and assert McpStrCpyN never disturbs the guard + * (never writes at or beyond dst[n]). */ +PROP_TEST(strutil_bounded) { + char src[MAX_SRC + 1]; + char buf[MAX_SRC + 2]; + int n; + + gen_src(_pc, src); + n = PROP_INT(1, MAX_SRC); + memset(buf, 'Z', sizeof(buf)); + buf[n] = (char)0xAB; /* guard byte right after the n-byte dst */ + McpStrCpyN(buf, src, n); + PROP_CHECK((unsigned char)buf[n] == 0xAB); /* guard intact */ + PROP_CHECK((int)strlen(buf) <= n - 1); /* NUL within the n bytes */ +} + +/* P5: edges - dst == NULL returns NULL; n <= 0 writes nothing. */ +PROP_TEST(strutil_edges) { + char src[MAX_SRC + 1]; + char buf[4]; + int n; + char *r; + + gen_src(_pc, src); + + /* dst == NULL returns NULL for any n. */ + n = PROP_INT(-8, MAX_SRC); + r = McpStrCpyN(NULL, src, n); + PROP_CHECK(r == NULL); + + /* n <= 0 writes nothing and returns dst unchanged. */ + buf[0] = (char)0xAB; + n = PROP_INT(-8, 0); /* in -8..0 */ + r = McpStrCpyN(buf, src, n); + PROP_CHECK(r == buf); + PROP_CHECK((unsigned char)buf[0] == 0xAB); +} + +int main(void) +{ + prop_seed(0); + + PROP_RUN(strutil_terminated, 2000); + PROP_RUN(strutil_prefix, 2000); + PROP_RUN(strutil_no_split, 2000); + PROP_RUN(strutil_bounded, 2000); + PROP_RUN(strutil_edges, 2000); + + return prop_summary(); +} From b333928370d23e802c067a1c023e94da4c01f263 Mon Sep 17 00:00:00 2001 From: David Connolly Date: Mon, 15 Jun 2026 23:51:06 +0100 Subject: [PATCH 5/7] Fail closed when a parent-only pipe end cannot be made non-inheritable The weed gate-bypass audit found ClearHandleInherit fails open: it dropped HANDLE_FLAG_INHERIT on the parent-only pipe ends but ignored failure on both routes (SetHandleInformation return discarded; DuplicateHandle fallback left the original inheritable handle on failure), so a child could inherit a parent-only pipe end. Make it return BOOL and abort the spawn (goto fail_pipes, before CreateProcessA) if any clear fails, rather than launching with an inheritable parent end. Never-path on a fresh handle, benign blast radius, but fail-closed matches the project's restraint posture. Tests: assert the success routes return TRUE; a stubbed SetHandleInformation returning FALSE is now DETECTED (failclosed_unit) and aborts ExecOpRun with "could not isolate parent pipe handles" before any child spawn (failclosed_spawn). Co-Authored-By: Claude Opus 4.8 --- src/exec_ops.c | 31 +++++++++++------ src/exec_ops.h | 4 ++- tests/test_exec_ops.c | 80 ++++++++++++++++++++++++++++++++++++++----- 3 files changed, 96 insertions(+), 19 deletions(-) diff --git a/src/exec_ops.c b/src/exec_ops.c index 087b17f..deacc5d 100644 --- a/src/exec_ops.c +++ b/src/exec_ops.c @@ -136,32 +136,38 @@ static void SetMsg(char *errMsg, int errSize, const char *s) * 3.51/Win95), so when the runtime probe found it absent we fall back to the * classic pre-3.51 idiom: duplicate the handle non-inheritable and drop the * inheritable original. DuplicateHandle is present since NT 3.1. + * + * Fails closed: returns TRUE iff the handle is guaranteed non-inheritable + * afterward. A discarded SetHandleInformation/DuplicateHandle failure would + * leave an inheritable parent end the child could leak, so the caller aborts + * the spawn when this returns FALSE. */ -static void ClearHandleInherit(HANDLE *ph) +static BOOL ClearHandleInherit(HANDLE *ph) { HANDLE dup; if (ph == NULL || *ph == NULL || *ph == INVALID_HANDLE_VALUE) { - return; + return TRUE; } if (g_features.pSetHandleInformation != NULL) { - g_features.pSetHandleInformation(*ph, HANDLE_FLAG_INHERIT, 0); - return; + return g_features.pSetHandleInformation(*ph, HANDLE_FLAG_INHERIT, 0); } if (DuplicateHandle(GetCurrentProcess(), *ph, GetCurrentProcess(), &dup, 0, FALSE, DUPLICATE_SAME_ACCESS)) { CloseHandle(*ph); *ph = dup; + return TRUE; } + return FALSE; } #ifdef TEST_BUILD /* Test-only hook: invoke the static ClearHandleInherit so test_exec_ops can * pin both the SetHandleInformation route and the NT 3.1 DuplicateHandle * fallback (forced by NULLing g_features.pSetHandleInformation). */ -void ExecClearHandleInheritForTest(HANDLE *ph) +BOOL ExecClearHandleInheritForTest(HANDLE *ph) { - ClearHandleInherit(ph); + return ClearHandleInherit(ph); } #endif @@ -411,10 +417,15 @@ int ExecOpRun( SetMsg(errMsg, errSize, "pipe creation failed"); goto fail_pipes; } - /* Q5: child must not inherit the parent-only ends. */ - ClearHandleInherit(&inWr); - ClearHandleInherit(&outRd); - ClearHandleInherit(&errRd); + /* Q5: child must not inherit the parent-only ends. Fail closed - if the + * inherit flag cannot be dropped, do NOT spawn with an inheritable parent + * end (a child handle leak); abort the open instead. */ + if (!ClearHandleInherit(&inWr) || + !ClearHandleInherit(&outRd) || + !ClearHandleInherit(&errRd)) { + SetMsg(errMsg, errSize, "spawn failed: could not isolate parent pipe handles"); + goto fail_pipes; + } memset(&si, 0, sizeof(si)); si.cb = sizeof(si); diff --git a/src/exec_ops.h b/src/exec_ops.h index 7c0a9bb..a4a54ff 100644 --- a/src/exec_ops.h +++ b/src/exec_ops.h @@ -116,8 +116,10 @@ int ExecOpRun( * ClearHandleInherit, so test_exec_ops can pin BOTH routes: the * SetHandleInformation path and the NT 3.1 DuplicateHandle fallback (reached * by forcing g_features.pSetHandleInformation to NULL in the test). + * Returns TRUE iff the handle is guaranteed non-inheritable afterward + * (the helper fails closed). */ -void ExecClearHandleInheritForTest(HANDLE *ph); +BOOL ExecClearHandleInheritForTest(HANDLE *ph); #endif #endif /* EXEC_OPS_H */ diff --git a/tests/test_exec_ops.c b/tests/test_exec_ops.c index 480ffc2..80cb635 100644 --- a/tests/test_exec_ops.c +++ b/tests/test_exec_ops.c @@ -724,6 +724,7 @@ TEST_CASE(clear_inherit_fallback) SetHandleInfoFn saved; HANDLE h; DWORD flags; + BOOL ok; saved = g_features.pSetHandleInformation; h = make_inheritable_handle(); @@ -734,9 +735,11 @@ TEST_CASE(clear_inherit_fallback) /* Force the NT 3.1 fallback path. */ g_features.pSetHandleInformation = NULL; - ExecClearHandleInheritForTest(&h); + ok = ExecClearHandleInheritForTest(&h); g_features.pSetHandleInformation = saved; /* restore */ + TEST_ASSERT(ok, "fallback route reported success"); + TEST_ASSERT(h != NULL && h != INVALID_HANDLE_VALUE, "fallback left a valid handle"); flags = 0; @@ -752,6 +755,7 @@ TEST_CASE(clear_inherit_api) { HANDLE h; DWORD flags; + BOOL ok; if (g_features.pSetHandleInformation == NULL) { TEST_ASSERT(1, "skipped: host has no SetHandleInformation probe"); @@ -763,7 +767,8 @@ TEST_CASE(clear_inherit_api) TEST_ASSERT(GetHandleInformation(h, &flags) != 0, "read initial flags"); TEST_ASSERT((flags & HANDLE_FLAG_INHERIT) != 0, "handle starts inheritable"); - ExecClearHandleInheritForTest(&h); /* probe present -> API route */ + ok = ExecClearHandleInheritForTest(&h); /* probe present -> API route */ + TEST_ASSERT(ok, "API route reported success"); TEST_ASSERT(h != NULL && h != INVALID_HANDLE_VALUE, "handle still valid"); flags = 0; @@ -787,21 +792,78 @@ TEST_CASE(clear_inherit_guards) if (route == 1) { g_features.pSetHandleInformation = NULL; } - /* NULL ph: no crash, returns. */ - ExecClearHandleInheritForTest(NULL); - /* NULL handle: no crash. */ + /* NULL ph: no crash, vacuously safe -> TRUE. */ + TEST_ASSERT(ExecClearHandleInheritForTest(NULL), + "NULL ph vacuously safe (TRUE)"); + /* NULL handle: no crash, vacuously safe -> TRUE. */ h = NULL; - ExecClearHandleInheritForTest(&h); + TEST_ASSERT(ExecClearHandleInheritForTest(&h), + "NULL handle vacuously safe (TRUE)"); TEST_ASSERT(h == NULL, "NULL handle left unchanged"); - /* INVALID_HANDLE_VALUE: no crash. */ + /* INVALID_HANDLE_VALUE: no crash, vacuously safe -> TRUE. */ h = INVALID_HANDLE_VALUE; - ExecClearHandleInheritForTest(&h); + TEST_ASSERT(ExecClearHandleInheritForTest(&h), + "INVALID handle vacuously safe (TRUE)"); TEST_ASSERT(h == INVALID_HANDLE_VALUE, "INVALID handle left unchanged"); } g_features.pSetHandleInformation = saved; /* restore */ TEST_ASSERT(1, "guards survived both routes"); } +/* A SetHandleInformation that always fails - drives the fail-closed path. */ +static BOOL WINAPI fail_set_handle_info(HANDLE h, DWORD m, DWORD f) +{ + (void)h; (void)m; (void)f; + SetLastError(ERROR_INVALID_FUNCTION); + return FALSE; +} + +/* ClearHandleInherit fails CLOSED: a failing SetHandleInformation is detected + * (returns FALSE), not swallowed. */ +TEST_CASE(clear_inherit_failclosed_unit) +{ + SetHandleInfoFn saved; + HANDLE h; + BOOL ok; + + saved = g_features.pSetHandleInformation; + h = make_inheritable_handle(); + TEST_ASSERT(h != NULL && h != INVALID_HANDLE_VALUE, "made inheritable handle"); + + g_features.pSetHandleInformation = fail_set_handle_info; + ok = ExecClearHandleInheritForTest(&h); + g_features.pSetHandleInformation = saved; /* restore */ + + TEST_ASSERT(!ok, "SetHandleInformation failure is DETECTED, not swallowed"); + CloseHandle(h); +} + +/* Integration pin: when the inherit flag cannot be dropped, ExecOpRun aborts + * at the handle-isolation step BEFORE CreateProcessA - it returns 0 (failure) + * and the errMsg names the isolation failure. (Aborting pre-spawn also keeps + * this case off the wine cmd.exe divergence the other exec cases hit.) */ +TEST_CASE(clear_inherit_failclosed_spawn) +{ + SetHandleInfoFn saved; + ExecResult r; + char msg[128]; + int ok; + + saved = g_features.pSetHandleInformation; + clear_bufs(); + msg[0] = '\0'; + + g_features.pSetHandleInformation = fail_set_handle_info; + ok = ExecOpRun("cmd /c echo hello", NULL, T_TIMEOUT, 1, + NULL, 0, g_out, sizeof(g_out), g_err, sizeof(g_err), + 0, 0, BIN_PE32, &r, msg, sizeof(msg)); + g_features.pSetHandleInformation = saved; /* restore */ + + TEST_ASSERT(!ok, "spawn aborts when parent ends cannot be isolated"); + TEST_ASSERT(strstr(msg, "isolate") != NULL, + "errMsg names the fail-closed isolation step"); +} + int main(void) { /* Unbuffered: a hang mid-suite must not swallow progress output. */ @@ -845,6 +907,8 @@ int main(void) RUN_TEST(clear_inherit_fallback); RUN_TEST(clear_inherit_api); RUN_TEST(clear_inherit_guards); + RUN_TEST(clear_inherit_failclosed_unit); + RUN_TEST(clear_inherit_failclosed_spawn); print_test_summary(); return g_tests_failed; From b4719be2ff2752ee2eec12992d385a8bf7d05ce7 Mon Sep 17 00:00:00 2001 From: David Connolly Date: Mon, 15 Jun 2026 23:56:30 +0100 Subject: [PATCH 6/7] Reconcile the Win32s import allowlist with the NT-floor fixes CI's function-level allowlist check has been red since the NT-floor work because the import set drifted: F2 made SetHandleInformation a GetProcAddress probe (no longer a static import) and added DuplicateHandle (fallback); F3 dropped lstrcpynA for McpStrCpyN; the COM1 fix added GetCommState. Update the allowlist to match exactly: +DuplicateHandle +GetCommState, -SetHandleInformation -lstrcpynA. Both additions verified present in the Win32s 1.25a thunk export surface (W32SCOMB.DLL exports GetCommState[697] and DuplicateHandle[2015], alongside the already-listed SetCommState/SetCommTimeouts), per the file header's baseline-image method. Local replication of the CI check: 58 imports, zero drift. Co-Authored-By: Claude Opus 4.8 --- tools/win32s-import-allowlist.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/win32s-import-allowlist.txt b/tools/win32s-import-allowlist.txt index a677330..2ddfefd 100644 --- a/tools/win32s-import-allowlist.txt +++ b/tools/win32s-import-allowlist.txt @@ -40,6 +40,7 @@ CreateProcessA CreateThread DeleteCriticalSection DeleteFileA +DuplicateHandle EnterCriticalSection ExitProcess FindClose @@ -48,6 +49,7 @@ FindNextFileA FlushFileBuffers GetACP GetCommandLineA +GetCommState GetCurrentProcess GetExitCodeProcess GetFileSize @@ -75,7 +77,6 @@ SetCommState SetCommTimeouts SetErrorMode SetFilePointer -SetHandleInformation Sleep TerminateProcess VirtualQuery @@ -85,6 +86,5 @@ lstrcatA lstrcmpA lstrcmpiA lstrcpyA -lstrcpynA lstrlenA wsprintfA From 879d2045628b30083ad6ec2616049bd5a1a0add0 Mon Sep 17 00:00:00 2001 From: David Connolly Date: Tue, 16 Jun 2026 00:26:14 +0100 Subject: [PATCH 7/7] Bound the \\.\COMn device-name build (review observation O1) The \\.\COMn fallback used an unbounded wsprintfA into devName[40], safe only by the implicit port[32] cross-module bound. Build it explicitly bounded instead: lstrcpyA the fixed 4-char prefix, then McpStrCpyN the port name into the remainder (sizeof(devName)-4), independent of the caller's buffer size. No import change (lstrcpyA/McpStrCpyN already used); allowlist still exact. Co-Authored-By: Claude Opus 4.8 --- src/serial.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/serial.c b/src/serial.c index 7909657..fdbcda7 100644 --- a/src/serial.c +++ b/src/serial.c @@ -102,7 +102,11 @@ HANDLE OpenSerialPort(const char *portName, DWORD baudRate) if (hPort == INVALID_HANDLE_VALUE && !(portName[0] == '\\' && portName[1] == '\\')) { char devName[40]; - wsprintfA(devName, "\\\\.\\%s", portName); + /* Bounded build of "\\.\" - the 4-char literal prefix then a + * length-limited copy of the name into the remainder, so this does not + * depend on the caller's port[] size (wsprintfA is unbounded). */ + lstrcpyA(devName, "\\\\.\\"); + McpStrCpyN(devName + 4, portName, (int)sizeof(devName) - 4); hPort = CreateFileA(devName, GENERIC_READ | GENERIC_WRITE, 0, NULL, OPEN_EXISTING, 0, NULL);