From 644c8fd5e88e39161ddfd373c23681bebe99d2d7 Mon Sep 17 00:00:00 2001
From: Martin Vogel <martin.vogel.tech@gmail.com>
Date: Fri, 31 Jul 2026 02:15:48 +0200
Subject: [PATCH] fix(mcp): bound the help-list appends instead of trusting
 snprintf's return

CodeQL flagged two high-severity alerts in the previous commit: the help-list
builder accumulated snprintf's return value, which is the length it WOULD have
written rather than what it did. Once len passes cap, the next `cap - len`
underflows to a huge size_t and the following write lands outside the buffer.

That is the same class the Cypher UNWIND list builder was just fixed for in
#1173, where blen advanced past a 2KB buffer the same way.

The capacity computed here is in fact sufficient for the current registry, so
nothing overflows today -- which makes this the more dangerous shape, not the
safer one. The code was correct only by an argument made ten lines away in the
cap loop, so renaming a tool or changing the wrap rule would have turned it into
an out-of-bounds write with nothing in the function to notice.

help_append() returns the bytes ACTUALLY written, clamped to the space left, so
`len <= cap - 1` becomes a local invariant that holds no matter what the
registry or the wrap column later become. Also adds the <stdarg.h> the variadic
helper needs.

Verified: builds clean under -Werror; --help still renders all 15 tools in
registry order with the same wrapping; mcp + cli suites 437 passed, 0 failed;
make -f Makefile.cbm lint-ci clean.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
---
 src/mcp/mcp.c | 36 ++++++++++++++++++++++++++++++++----
 1 file changed, 32 insertions(+), 4 deletions(-)

diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c
index 3f79441c2..171f0dab2 100644
--- a/src/mcp/mcp.c
+++ b/src/mcp/mcp.c
@@ -89,6 +89,7 @@ enum {
 #include <yyjson/yyjson.h>
 #include <ctype.h>
 #include <limits.h>
+#include <stdarg.h> // va_list, for the bounded help-list appender
 #include <stdint.h> // int64_t
 #include <stdio.h>
 #include <stdlib.h>
@@ -906,6 +907,32 @@ const char *cbm_mcp_tool_name(int index) {
  * serves. The list used to be hand-maintained in the help text and drifted
  * when check_index_coverage was added (#1361); deriving it here makes that
  * divergence impossible. Heap-allocated; caller frees. */
+/* Append at out[len] and return the bytes ACTUALLY written.
+ *
+ * snprintf returns the length it WOULD have written, so accumulating that value
+ * lets len run past cap; the next `cap - len` then underflows to a huge size_t
+ * and the following write lands outside the buffer. CodeQL flagged exactly that
+ * shape here, and it is the same class #1173 just fixed in the Cypher list
+ * builder. The capacity computed below does happen to be sufficient today —
+ * which makes this the more dangerous version, not the safer one: the code is
+ * correct only by an argument made ten lines away, so renaming a tool or
+ * changing the wrap rule would turn it into an overflow with nothing to notice.
+ * Clamping makes `len <= cap - 1` a local invariant no later edit can void. */
+static size_t help_append(char *out, size_t cap, size_t len, const char *fmt, ...) {
+    if (len + 1 >= cap) {
+        return 0;
+    }
+    va_list args;
+    va_start(args, fmt);
+    int written = vsnprintf(out + len, cap - len, fmt, args);
+    va_end(args);
+    if (written <= 0) {
+        return 0;
+    }
+    size_t room = cap - len - 1;
+    return (size_t)written > room ? room : (size_t)written;
+}
+
 char *cbm_mcp_tools_help_list(void) {
     size_t cap = SLEN("Tools:") + 2; /* trailing newline + NUL */
     for (int i = 0; i < TOOL_COUNT; i++) {
@@ -915,20 +942,21 @@ char *cbm_mcp_tools_help_list(void) {
     if (!out) {
         return NULL;
     }
-    size_t len = (size_t)snprintf(out, cap, "Tools:");
+    size_t len = help_append(out, cap, 0, "Tools:");
     size_t col = len;
     for (int i = 0; i < TOOL_COUNT; i++) {
         const char *sep = (i + 1 < TOOL_COUNT) ? "," : "";
         size_t item = SLEN(" ") + strlen(TOOLS[i].name) + strlen(sep);
         if (i > 0 && col + item > MCP_HELP_TOOLS_WRAP_COL) {
-            len += (size_t)snprintf(out + len, cap - len, "\n ");
+            len += help_append(out, cap, len, "\n ");
             col = 1;
         }
-        size_t wrote = (size_t)snprintf(out + len, cap - len, " %s%s", TOOLS[i].name, sep);
+        size_t wrote = help_append(out, cap, len, " %s%s", TOOLS[i].name, sep);
         len += wrote;
         col += wrote;
     }
-    snprintf(out + len, cap - len, "\n");
+    len += help_append(out, cap, len, "\n");
+    (void)len; /* final length is not needed; the buffer is NUL-terminated */
     return out;
 }