Skip to content

fix(scan): size build_scan_json buffer from token text length, never write past it - #168

Merged
pyramation merged 1 commit into
mainfrom
fix/scan-json-buffer-overflow
Sep 17, 2026
Merged

pyramation merged 1 commit into
mainfrom
fix/scan-json-buffer-overflow

Conversation

@pyramation

Copy link
Copy Markdown
Collaborator

Summary

Fixes #167.

build_scan_json() (v18 only) sized its output buffer from the token count (1024 + 200*n_tokens) and only grew it after a bounded snprintf had already clipped a write. Because pos accumulated snprintf's would-have-written length, pos could exceed estimated_size, making estimated_size - pos underflow and the next snprintf write past the allocation.

Change (in templates/full/wasm_wrapper.c, propagated to versions/18/src/wasm_wrapper.c):

  • Size the buffer once, up front, from token text length: 1024 + Σ (token_len*2 + 200) — 2× covers worst-case JSON escaping, 200 covers the fixed per-token JSON (max ~170 bytes incl. commas).
  • Delete the grow-after-write realloc path.
  • pos is now size_t, each append is guarded by pos < estimated_size, and pos is clamped to estimated_size - 1 so estimated_size - pos can never underflow even if the estimate were ever wrong.

Regression tests added to versions/18/test/scan.test.js: 1400 / 5000 / 300000-char literals followed by more statements, and long dollar-quoted tokens full of " \ \n \t. Verified the new tests fail on published libpg-query@18.1.4 (Unterminated string in JSON at position 1623) and pass on the rebuilt WASM; the issue's Part 2 heap-clobber probe reports no clobber against the rebuilt module.

Link to Devin session: https://app.devin.ai/sessions/332ad0c592c8445dae0d0e498973ca79
Open in Devin Desktop: https://app.devin.ai/desktop/session/332ad0c592c8445dae0d0e498973ca79?variant=devin
Requested by: @pyramation

@devin-ai-integration

Copy link
Copy Markdown

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@pyramation
pyramation merged commit c8cb586 into main Sep 17, 2026
59 of 60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant