sql(mysql): bounds-check params in bind() when values array is mutated

src/sql/mysql/MySQLQuery.zig

@@ -24,6 +24,13 @@ fn bind(this: *MySQLQuery, execute: *PreparedStatement.Execute, globalObject: *J
         bun.default_allocator.free(params);
     }
     while (try iter.next()) |js_value| {
+        if (i >= params.len) {
+            // The binding array yielded more values than the prepared statement
+            // expects. This can happen when the user-supplied array is mutated (e.g.
+            // from an index getter) between signature generation and binding. Fail
+            // loudly instead of writing past the end of `params`/`param_types`.
+            return error.WrongNumberOfParametersProvided;
+        }
         const param = execute.param_types[i];
         params[i] = try Value.fromJS(
             js_value,
@@ -38,6 +45,12 @@ fn bind(this: *MySQLQuery, execute: *PreparedStatement.Execute, globalObject: *J
         return error.InvalidQueryBinding;
     }
 
+    if (i != params.len) {
+        // Fewer values than the prepared statement expects; the remaining slots
+        // would be uninitialized.
+        return error.WrongNumberOfParametersProvided;
+    }
+
     this.#status = .binding;
     execute.params = params;
 }
@@ -47,18 +60,21 @@ fn bindAndExecute(this: *MySQLQuery, writer: anytype, statement: *MySQLStatement
     if (statement.signature.fields.len != statement.params.len) {
         return error.WrongNumberOfParametersProvided;
     }
-    var packet = try writer.start(0);
     var execute = PreparedStatement.Execute{
         .statement_id = statement.statement_id,
         .param_types = statement.signature.fields,
         .new_params_bind_flag = statement.execution_flags.need_to_send_params,
         .iteration_count = 1,
     };
-    statement.execution_flags.need_to_send_params = false;
     defer execute.deinit();
+    // Bind before touching the writer so a bind failure (user-triggerable via JS
+    // getters / param-count mismatch) doesn't leave a partial packet header in
+    // the connection's write buffer.
     try this.bind(&execute, globalObject, binding_value, columns_value);
+    var packet = try writer.start(0);
     try execute.write(writer);
     try packet.end();
+    statement.execution_flags.need_to_send_params = false;
     this.#status = .running;
 }
 

test/js/sql/sql-mysql-bind-oob.fixture.ts

@@ -0,0 +1,49 @@
+// Reproducer for an out-of-bounds write in MySQLQuery.bind().
+//
+// Signature.generate() and bind() each create a fresh iterator over the
+// user-supplied params array. If an index getter mutates the array so that
+// the second iteration is longer than the first, bind() would index past the
+// `params` / `param_types` buffers it sized based on the first iteration.
+//
+// Without the bounds check this panics in debug builds (index out of bounds)
+// and is a silent heap overflow in release builds.
+
+import { SQL } from "bun";
+
+const url = process.env.MYSQL_URL;
+if (!url) throw new Error("MYSQL_URL is required");
+
+const tls = process.env.CA_PATH ? { ca: Bun.file(process.env.CA_PATH) } : undefined;
+const sql = new SQL({ url, tls, max: 1 });
+
+try {
+  // Prime the prepared-statement cache so the next call with the same
+  // signature goes straight to bindAndExecute without re-preparing.
+  await sql.unsafe("select ? as x", [1]);
+
+  const values: number[] = [1];
+  let fired = 0;
+  Object.defineProperty(values, "0", {
+    enumerable: true,
+    configurable: true,
+    get() {
+      if (fired++ === 0) {
+        for (let i = 0; i < 100; i++) values.push(1);
+      }
+      return 1;
+    },
+  });
+
+  const result = await sql.unsafe("select ? as x", values).then(
+    rows => ({ ok: true, rows }),
+    err => ({ ok: false, code: err?.code, message: String(err?.message ?? err) }),
+  );
+
+  // The connection must still be usable after the bind failure; a partial
+  // packet header left in the write buffer would desync the protocol here.
+  const after = await sql.unsafe("select ? as x", [2]);
+
+  console.log(JSON.stringify({ result, after }));
+} finally {
+  await sql.close();
+}

test/js/sql/sql-mysql.test.ts

@@ -1,6 +1,6 @@
 import { SQL, randomUUIDv7 } from "bun";
 import { beforeAll, describe, expect, mock, test } from "bun:test";
-import { bunEnv, bunRun, describeWithContainer, isDockerEnabled, tempDirWithFiles } from "harness";
+import { bunEnv, bunExe, bunRun, describeWithContainer, isDockerEnabled, tempDirWithFiles } from "harness";
 import net from "net";
 import path from "path";
 const dir = tempDirWithFiles("sql-test", {
@@ -817,6 +817,44 @@ if (isDockerEnabled()) {
           expect(await sql.unsafe("select 1 as x")).toEqual([{ x: 1 }]);
         });
 
+        test("unsafe does not OOB when the params array grows during binding", async () => {
+          // Signature generation and binding each iterate the user-supplied params
+          // array. If an index getter mutates the array so that the second
+          // iteration is longer than the first, bind() must not read/write past
+          // the param buffer it allocated based on the first iteration's length.
+          // Run in a subprocess so a crash doesn't take down the rest of the suite.
+          await using proc = Bun.spawn({
+            cmd: [bunExe(), path.join(import.meta.dir, "sql-mysql-bind-oob.fixture.ts")],
+            env: {
+              ...bunEnv,
+              MYSQL_URL: String(getOptions().url),
+              CA_PATH: image.name === "MySQL with TLS" ? path.join(import.meta.dir, "mysql-tls", "ssl", "ca.pem") : "",
+            },
+            stdout: "pipe",
+            stderr: "pipe",
+          });
+          const [stdout, stderr, exitCode] = await Promise.all([
+            proc.stdout.text(),
+            proc.stderr.text(),
+            proc.exited,
+          ]);
+          const filteredStderr = stderr
+            .split(/\r?\n/)
+            .filter(l => l && !l.startsWith("WARNING: ASAN interferes"))
+            .join("\n");
+          expect(filteredStderr).toBe("");
+          expect(JSON.parse(stdout.trim())).toEqual({
+            result: {
+              ok: false,
+              code: "ERR_MYSQL_WRONG_NUMBER_OF_PARAMETERS_PROVIDED",
+              message: expect.any(String),
+            },
+            // Connection must remain usable after the bind failure.
+            after: [{ x: 2 }],
+          });
+          expect(exitCode).toBe(0);
+        });
+
         test("simple query with multiple statements", async () => {
           await using sql = new SQL({ ...getOptions(), max: 1 });
           const result = await sql`select 1 as x;select 2 as x`.simple();