Skip to content

fix(sqlite): bind template values positionally to prevent SQL injection - #35963

Closed
baima365-web wants to merge 1 commit into
oven-sh:mainfrom
baima365-web:fix-sqlite-positional-binding
Closed

fix(sqlite): bind template values positionally to prevent SQL injection#35963
baima365-web wants to merge 1 commit into
oven-sh:mainfrom
baima365-web:fix-sqlite-positional-binding

Conversation

@baima365-web

Copy link
Copy Markdown

What does this PR do?

SQLiteQueryHandle.run was using $apply to forward positional parameters, which let attacker-controlled objects/arrays override trusted bindings.

Problem

const attackerId = JSON.parse('{"0": 2, "1": "bob"}'); // attacker-controlled
await sql`SELECT * FROM docs WHERE id = ${attackerId} AND tenant = ${serverTenant}`;
// before: [{id:2, tenant:"bob", body:"B-secret"}]  (trusted tenant filter overridden)

Fix

Check if values is array and bind positionally instead of using $apply. This prevents objects/arrays from being misinterpreted as named binding maps.

Fixes #35950

SQLiteQueryHandle.run was using $apply to forward positional parameters,
which let attacker-controlled objects/arrays override trusted bindings.

Fix: Check if values is array and bind positionally instead of using $apply.

Fixes oven-sh#35950

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@robobun

robobun commented Aug 15, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the report and for the fix. The same change landed on main in #35950: SQLiteQueryHandle.run now hands the positional values to bun:sqlite as a single array instead of spreading them, so an object or array interpolated as a value can no longer rebind the other parameters (the query in your example now throws bun:sqlite's "Binding expected string, TypedArray, boolean, number, bigint or null" TypeError). That PR also added tests for the tagged template, sql.unsafe() and sql() helper paths. It will be in the next release.

Closing this one as superseded by #35950.

@robobun robobun closed this Aug 15, 2026
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.

2 participants