fix: harden SQL literal boundaries

src/placeholders.ts

@@ -3,12 +3,15 @@ export interface PlaceholderRewriteOptions {
   readonly sqlite?: boolean;
 }
 
-export function rewritePlaceholders(
+interface SqlStructuralReplacement {
+  readonly text: string;
+  readonly length: number;
+}
+
+export function rewriteStructuralSql(
   text: string,
-  values: readonly unknown[],
-  options: PlaceholderRewriteOptions,
-): { readonly text: string; readonly values: readonly unknown[] } {
-  const rewrittenValues: unknown[] = [];
+  replace: (source: string, index: number) => SqlStructuralReplacement | undefined,
+): string {
   let output = "";
   let index = 0;
   let quote: "'" | '"' | null = null;
@@ -59,8 +62,10 @@ export function rewritePlaceholders(
       blockComment = true;
       continue;
     }
-    if (options.sqlite && text.startsWith('"public".', index)) {
-      index += 9;
+    const replacement = replace(text, index);
+    if (replacement) {
+      output += replacement.text;
+      index += replacement.length;
       continue;
     }
     const character = text[index]!;
@@ -71,21 +76,6 @@ export function rewritePlaceholders(
       continue;
     }
     if (character === "$") {
-      const placeholder = text.slice(index).match(/^\$(\d+)/);
-      if (placeholder) {
-        const position = Number(placeholder[1]);
-        if (position < 1 || position > values.length) {
-          throw new Error(`SQL placeholder $${position} has no matching value.`);
-        }
-        if (options.sqlite) {
-          output += "?";
-          rewrittenValues.push(values[position - 1]);
-        } else {
-          output += `$${position + (options.offset ?? 0)}`;
-        }
-        index += placeholder[0].length;
-        continue;
-      }
       const delimiter = text.slice(index).match(/^\$[A-Za-z_][A-Za-z0-9_]*\$|^\$\$/)?.[0];
       if (delimiter) {
         dollarQuote = delimiter;
@@ -97,6 +87,31 @@ export function rewritePlaceholders(
     output += character;
     index += 1;
   }
+  return output;
+}
+
+export function rewritePlaceholders(
+  text: string,
+  values: readonly unknown[],
+  options: PlaceholderRewriteOptions,
+): { readonly text: string; readonly values: readonly unknown[] } {
+  const rewrittenValues: unknown[] = [];
+  const output = rewriteStructuralSql(text, (source, index) => {
+    if (options.sqlite && source.startsWith('"public".', index)) {
+      return { text: "", length: 9 };
+    }
+    const placeholder = source.slice(index).match(/^\$(\d+)/);
+    if (!placeholder) return undefined;
+    const position = Number(placeholder[1]);
+    if (position < 1 || position > values.length) {
+      throw new Error(`SQL placeholder $${position} has no matching value.`);
+    }
+    if (options.sqlite) rewrittenValues.push(values[position - 1]);
+    return {
+      text: options.sqlite ? "?" : `$${position + (options.offset ?? 0)}`,
+      length: placeholder[0].length,
+    };
+  });
   return { text: output, values: options.sqlite ? rewrittenValues : values };
 }
 

src/sql.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, it } from "vitest";
-import { and, compileKeyedSql, compileSql, eq, identifier, inArray, sql } from "./index";
+import { and, compileKeyedSql, compileSql, eq, identifier, inArray, literal, sql } from "./index";
 import { rewritePlaceholders, sqlStructure } from "./placeholders";
 
 describe("SQL boundaries", () => {
@@ -47,4 +47,43 @@ describe("SQL boundaries", () => {
     });
     expect(() => sql.key("bad key", {})``).toThrow(/Invalid keyed SQL key/);
   });
+
+  it("should replace only structural named parameters given inert SQL regions", () => {
+    const exact = sql.key("notes.search", { email: "" })`
+      SELECT id FROM users WHERE note = 'contact via :email for help' AND email = :email
+    `;
+    expect(compileKeyedSql(exact, { email: "attacker@example.com" })).toEqual({
+      text: "\n      SELECT id FROM users WHERE note = 'contact via :email for help' AND email = $1\n    ",
+      values: ["attacker@example.com"],
+    });
+
+    let seed = 0x5eed;
+    for (let sample = 0; sample < 100; sample += 1) {
+      seed = (seed * 16_807) % 2_147_483_647;
+      const name = `p_${seed.toString(36)}`;
+      const inert = `:${name}`;
+      const source = [
+        `SELECT '${inert}', "quoted ${inert}", $$${inert}$$, $tag$${inert}$tag$`,
+        `-- ${inert}`,
+        `/* ${inert} */ WHERE id = :${name}`,
+      ].join("\n");
+      const query = {
+        kind: "keyed-sql" as const,
+        key: `guardrail.${sample}`,
+        source,
+        parameters: { [name]: 0 },
+      };
+
+      expect(compileKeyedSql(query, { [name]: sample })).toEqual({
+        text: source.replace(`WHERE id = :${name}`, "WHERE id = $1"),
+        values: [sample],
+      });
+    }
+  });
+
+  it("should reject non-finite numbers given SQL literal formatting", () => {
+    for (const value of [Number.NaN, Number.POSITIVE_INFINITY, Number.NEGATIVE_INFINITY]) {
+      expect(() => compileSql(sql`SELECT ${literal(value)}`)).toThrow(/finite number/i);
+    }
+  });
 });

src/sql.ts

@@ -2,6 +2,7 @@ import { quoteIdentifier } from "./naming";
 import type { DatabaseAdapter, QueryOptions } from "./adapter";
 import { normalizeDatabaseError } from "./errors";
 import type { AnyTable } from "./schema";
+import { rewriteStructuralSql } from "./placeholders";
 
 const SQL_FRAGMENT = Symbol("askr.sql.fragment");
 
@@ -57,12 +58,18 @@ export function identifier(name: string): SqlFragment {
   return fragment([{ kind: "identifier", value: name }]);
 }
 
-/** Embeds a value as a SQL literal (not a bound parameter). Also available as `sql.literal`. */
+/**
+ * Embeds a value as a SQL literal (not a bound parameter). Also available as `sql.literal`.
+ * @throws {RangeError} If a numeric value is not finite.
+ */
 export function literal(value: string | number | boolean | null): SqlFragment {
   if (typeof value === "string") {
     return fragment([{ kind: "text", value: `'${value.replaceAll("'", "''")}'` }]);
   }
   if (value === null) return fragment([{ kind: "text", value: "NULL" }]);
+  if (typeof value === "number" && !Number.isFinite(value)) {
+    throw new RangeError("SQL numeric literals require a finite number.");
+  }
   return fragment([{ kind: "text", value: String(value) }]);
 }
 
@@ -289,7 +296,10 @@ export function compileKeyedSql(
 ): SqlQuery {
   const ordered: unknown[] = [];
   const positions = new Map<string, number>();
-  const text = query.source.replace(/(?<!:):([a-z_][a-z0-9_]*)/gi, (_match, name: string) => {
+  const text = rewriteStructuralSql(query.source, (source, index) => {
+    const placeholder = source.slice(index).match(/^:([a-z_][a-z0-9_]*)/i);
+    if (!placeholder || source[index - 1] === ":") return undefined;
+    const name = placeholder[1]!;
     if (!(name in query.parameters)) {
       throw new Error(`Keyed SQL ${query.key} uses undeclared parameter :${name}.`);
     }
@@ -302,7 +312,7 @@ export function compileKeyedSql(
       position = ordered.length;
       positions.set(name, position);
     }
-    return `$${position}`;
+    return { text: `$${position}`, length: placeholder[0].length };
   });
   return { text, values: ordered };
 }