Skip to content

scan(): build_scan_json() truncates output on large tokens, and writes out of bounds past the JSON buffer #167

Description

@ophir-docomply

Summary

scan() / scanSync() silently truncate their JSON output when the tokens are
large, so JSON.parse throws and the call fails. The cause is in
build_scan_json(): the output buffer is sized from a token count heuristic,
but grown only after a bounded snprintf has already clipped a write.

A second consequence of the same line is worse than the truncation: because
pos accumulates snprintf's would-have-written return value, pos can end
up past estimated_size. estimated_size - pos is then a size_t underflow,
so the next snprintf receives a huge bound and writes outside the
allocation
. I have an observable clobber of an unrelated live allocation
(Part 2 below).

Scope note up front: the write is inside the WASM module's own linear memory, so
it cannot escape the sandbox — it corrupts the module's heap, nothing more. I am
reporting it publicly on that basis (there is no SECURITY.md / private
advisory channel on the repo). It is still reachable from any caller that hands
scan() SQL containing a long token.

Affected: libpg-query@18.1.4 (pg18); the same code is in
versions/18/src/wasm_wrapper.c and templates/full/wasm_wrapper.c at main.
build_scan_json() only exists for v18, so 13–17 are not affected. parse() is
completely unaffected — the SQL below parses fine, only the scanner fails.

Root cause

https://github.com/constructive-io/libpg-query-node/blob/main/versions/18/src/wasm_wrapper.c

size_t estimated_size = 1024 + (scan_result->n_tokens * 200);   // token COUNT, not token SIZE
char* json = safe_malloc(estimated_size);
...
    pos += snprintf(json + pos, estimated_size - pos, "{...\"text\":\"%s\"...}", ..., escaped_text, ...);
    ...
    if (pos >= estimated_size - 200) {          // checked AFTER the write, and doubles only ONCE
        char* new_json = realloc(json, estimated_size * 2);
        ...
    }

Two distinct defects on that path:

  1. The estimate ignores token text length. A token's JSON contributes
    ~110 + 2 * token_length bytes, but the budget allots a flat 200 per token.
    Any token longer than ~90 characters (or ~45 if it needs JSON escaping)
    therefore exceeds its own allowance, and one long literal blows the whole
    budget.
  2. The buffer is grown after the write, not before. snprintf truncates to
    the bound it is given, so the data is already lost by the time the check
    runs — and the check only doubles once, which does not help when a single
    append is many times the capacity.

The truncation point is exactly 1024 + 200 * n_tokens - 1 bytes (the - 1 is
snprintf's NUL), which the repro confirms at three token counts.

Reproduction

npm i libpg-query@18.1.4 && node repro.mjs

import { scan } from 'libpg-query';
import createModule from 'libpg-query/wasm/libpg-query.js';

const m = await createModule();
const raw = (sql) => {
  const n = m.lengthBytesUTF8(sql) + 1;
  const q = m._malloc(n);
  m.stringToUTF8(sql, q, n);
  const r = m._wasm_scan(q);
  const s = m.UTF8ToString(r);
  m._free(q); m._wasm_free_string(r);
  return s;
};

// --- Part 1: output clipped to exactly 1024 + 200*n_tokens - 1 bytes
console.log('--- Part 1');
for (const [sql, tokens] of [
  [`SELECT '${'a'.repeat(5000)}';`, 3],
  [`SELECT 1; SELECT '${'a'.repeat(5000)}';`, 6],
  [`SELECT 1; SELECT 1; SELECT '${'a'.repeat(9000)}';`, 9],
]) {
  console.log(`  n_tokens=${tokens}  raw bytes=${raw(sql).length}  predicted=${1024 + 200 * tokens - 1}`);
}

// The user-visible symptom
try {
  await scan(`SELECT '${'a'.repeat(1400)}';`);
} catch (e) {
  console.log(`  scan(1400-char literal) throws -> ${e.message}`);
}

// --- Part 2: out-of-bounds write past the JSON buffer.
// Lay a contiguous run of sentinel chunks and free only the first, so the JSON
// buffer lands in that hole and anything written past its end hits a chunk that
// is still live.
console.log('--- Part 2');
const CH = 65536, N = 96, chunks = [];
for (let i = 0; i < N; i++) { const p = m._malloc(CH); m.HEAPU8.fill(0xab, p, p + CH); chunks.push(p); }
m._free(chunks[0]);
const watch = chunks.slice(1);
const before = watch.map((p) => m.HEAPU8.slice(p, p + CH));

raw(`SELECT '${'a'.repeat(300000)}'; SELECT 1; SELECT 2; SELECT 3;`);

for (let i = 0; i < watch.length; i++) {
  const now = m.HEAPU8.slice(watch[i], watch[i] + CH);
  for (let j = 0; j < CH; j++) {
    if (now[j] !== before[i][j]) {
      console.log(`  live allocation #${i + 1} clobbered at +${j}: ` +
        `0x${before[i][j].toString(16)} -> 0x${now[j].toString(16)} ` +
        `(${JSON.stringify(String.fromCharCode(now[j]))})`);
      process.exit(0);
    }
  }
}
console.log('  no clobber observed (allocator-layout dependent)');

Output on libpg-query@18.1.4, node 24:

--- Part 1
  n_tokens=3  raw bytes=1623  predicted=1623
  n_tokens=6  raw bytes=2223  predicted=2223
  n_tokens=9  raw bytes=2823  predicted=2823
  scan(1400-char literal) throws -> Unterminated string in JSON at position 1623 (line 1 column 1624)
--- Part 2
  live allocation #4 clobbered at +38091: 0xab -> 0x2c (",")

Part 2 is the "," separator from the token after the huge one, written
~300 KB past the buffer it belongs to. The exact chunk/offset depends on
allocator layout; the fact of the write does not.

Suggested fix

The token lengths are all known before the loop, so the buffer can be sized
exactly once and the grow-after-write path deleted entirely:

    /* Size from token TEXT length, not token count: each token contributes
       ~110 bytes of fixed JSON -- 200 leaves headroom -- plus up to 2 bytes per
       input byte once escaped. */
    size_t estimated_size = 1024;
    for (size_t i = 0; i < scan_result->n_tokens; i++) {
        PgQuery__ScanToken *t = scan_result->tokens[i];
        int len = t->end - t->start;
        if (len < 0) len = 0;
        estimated_size += (size_t) len * 2 + 200;
    }
    char* json = safe_malloc(estimated_size);

If you would rather keep a growing buffer, the load-bearing change is to grow
before each append and to loop rather than double once:

    size_t need = (size_t) escaped_pos + 256;
    while (estimated_size - (size_t) pos < need) {
        char* new_json = realloc(json, estimated_size * 2);
        if (!new_json) break;
        json = new_json;
        estimated_size *= 2;
    }
    pos += snprintf(json + pos, estimated_size - pos, "{...}", ...);

Either way, pos must never be allowed to exceed estimated_size, since every
later estimated_size - pos depends on it.

A regression test in the shape of #148 would pin it: scan a statement with a
literal a few KB long and assert the returned JSON parses.

Why this matters to us

We use parse() for AST-shaped questions and it has been flawless across a
~190-file, 3.6 MB SQL corpus. We need scan() for exactly one thing the AST
cannot answer — comments are discarded by the grammar, and we need to know which
byte ranges are inside a comment. A handful of real files in that corpus have
tokens over the budget, so that one migration is blocked on this. Happy to test
a fix against the corpus if that is useful.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions