Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions src/jsc/bindings/sqlite/JSSQLStatement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1211,6 +1211,13 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementSetCustomSQLite, (JSC::JSGlobalObject * l
return {};
}

String sqlitePath = sqliteStrValue.toWTFString(lexicalGlobalObject);
RETURN_IF_EXCEPTION(scope, {});
if (sqlitePath.find('\0') != WTF::notFound) [[unlikely]] {
throwException(lexicalGlobalObject, scope, createTypeError(lexicalGlobalObject, "The SQLite library path must not contain null bytes"_s));
return {};
}

#if LAZY_LOAD_SQLITE
if (sqlite3_handle) {
throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "SQLite already loaded\nThis function can only be called before SQLite has been loaded and exactly once. SQLite auto-loads when the first time you open a Database."_s));
Expand All @@ -1219,8 +1226,7 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementSetCustomSQLite, (JSC::JSGlobalObject * l

// Use a static CString to keep the string alive for the lifetime of the process
static CString sqlite3_lib_path_storage;
sqlite3_lib_path_storage = sqliteStrValue.toWTFString(lexicalGlobalObject).utf8();
RETURN_IF_EXCEPTION(scope, {});
sqlite3_lib_path_storage = sqlitePath.utf8();
sqlite3_lib_path = sqlite3_lib_path_storage.data();

if (lazyLoadSQLite() == -1) {
Expand Down Expand Up @@ -1413,6 +1419,18 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementLoadExtensionFunction, (JSC::JSGlobalObje
auto extensionString = extension.toWTFString(lexicalGlobalObject);
RETURN_IF_EXCEPTION(scope, {});

if (extensionString.find('\0') != WTF::notFound) [[unlikely]] {
throwException(lexicalGlobalObject, scope, createTypeError(lexicalGlobalObject, "The extension path must not contain null bytes"_s));
return {};
}
Comment thread
robobun marked this conversation as resolved.

auto entryPointStr = callFrame->argumentCount() > 2 && callFrame->argument(2).isString() ? callFrame->argument(2).toWTFString(lexicalGlobalObject) : String();
RETURN_IF_EXCEPTION(scope, {});
if (entryPointStr.find('\0') != WTF::notFound) [[unlikely]] {
throwException(lexicalGlobalObject, scope, createTypeError(lexicalGlobalObject, "The extension entry point must not contain null bytes"_s));
return {};
}

sqlite3* db = versionDB->handle();
if (!db) [[unlikely]] {
throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Can't do this on a closed database"_s));
Expand All @@ -1424,8 +1442,6 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementLoadExtensionFunction, (JSC::JSGlobalObje
return {};
}

auto entryPointStr = callFrame->argumentCount() > 2 && callFrame->argument(2).isString() ? callFrame->argument(2).toWTFString(lexicalGlobalObject) : String();
RETURN_IF_EXCEPTION(scope, {});
auto entryPointUtf8 = entryPointStr.utf8();
const char* entryPoint = entryPointStr.length() == 0 ? NULL : entryPointUtf8.data();
auto extensionStringUtf8 = extensionString.utf8();
Expand Down Expand Up @@ -1777,6 +1793,10 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementOpenStatementFunction, (JSC::JSGlobalObje
String path = pathValue.toWTFString(lexicalGlobalObject);
RETURN_IF_EXCEPTION(topExceptionScope, JSValue::encode(jsUndefined()));
(void)topExceptionScope.tryClearException();
if (path.find('\0') != WTF::notFound) [[unlikely]] {
throwException(lexicalGlobalObject, scope, createTypeError(lexicalGlobalObject, "The database path must not contain null bytes"_s));
return {};
}
int openFlags = DEFAULT_SQLITE_FLAGS;
if (callFrame->argumentCount() > 1) {
JSValue flags = callFrame->argument(1);
Expand Down
53 changes: 53 additions & 0 deletions test/js/bun/sqlite/sqlite.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2575,3 +2575,56 @@ it("exec/run with an embedded NUL byte in the SQL string does not hang", async (
exitCode: 0,
});
});

describe("paths with embedded null bytes", () => {
it("new Database() rejects them instead of truncating at the null byte", () => {
using dir = tempDir("sqlite-nul-path", {});
expect(() => new Database(path.join(String(dir), "db1\0zz.sqlite"))).toThrow(
"The database path must not contain null bytes",
);
// Without the guard, sqlite silently created the truncated path "db1".
expect(readdirSync(String(dir))).toEqual([]);
});

it("cannot open a different file than the one the JS string names", () => {
using dir = tempDir("sqlite-nul-suffix", {});
const real = path.join(String(dir), "real.sqlite");
{
const db = new Database(real);
db.run("CREATE TABLE s (v TEXT)");
db.run("INSERT INTO s VALUES ('secret')");
db.close();
}
// Without the guard, this opened real.sqlite even though the JS string
// ends with ".tmp".
expect(() => new Database(real + "\0.tmp", { readonly: true })).toThrow(
"The database path must not contain null bytes",
);
});

it("loadExtension() rejects them", () => {
const db = new Database(":memory:");
try {
expect(() => db.loadExtension("does-not-exist\0.so")).toThrow("The extension path must not contain null bytes");
} finally {
db.close();
}
});

it("loadExtension() rejects entry points containing them", () => {
const db = new Database(":memory:");
try {
expect(() => db.loadExtension("does-not-exist.so", "entry\0point")).toThrow(
"The extension entry point must not contain null bytes",
);
} finally {
db.close();
}
});

it("setCustomSQLite() rejects them", () => {
expect(() => Database.setCustomSQLite("libsqlite3\0.dylib")).toThrow(
"The SQLite library path must not contain null bytes",
);
});
});
23 changes: 21 additions & 2 deletions test/js/sql/sqlite-sql.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { randomUUIDv7, SQL } from "bun";
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, mock, test } from "bun:test";
import { tempDir } from "harness";
import { isDebug, tempDir } from "harness";
import { existsSync } from "node:fs";
import { rm, stat } from "node:fs/promises";
import { join } from "node:path";
Expand Down Expand Up @@ -2014,7 +2014,9 @@ describe("Memory and resource management", () => {

await sql`CREATE TABLE stmt_test (id INTEGER PRIMARY KEY, value TEXT)`;

const iterations = 10000;
// Debug+ASAN builds run 10-100x slower; 10k awaited inserts blow the
// default test timeout there.
const iterations = isDebug ? 1000 : 10000;

for (let i = 0; i < iterations; i++) {
await sql`INSERT INTO stmt_test (id, value) VALUES (${i}, ${"test" + i})`;
Expand Down Expand Up @@ -5403,3 +5405,20 @@ describe("Unicode & Encoding Fuzzing Tests", () => {
}
});
});

describe("filenames with embedded null bytes", () => {
test("are rejected instead of truncated at the null byte", async () => {
using dir = tempDir("sql-nul-path", {});
await using sql = new SQL({ adapter: "sqlite", filename: join(String(dir), "db3\0zz.sqlite") });
let error: Error | null = null;
try {
await sql`select 1`;
} catch (e) {
error = e as Error;
}
expect(error).not.toBeNull();
expect(error!.message).toContain("The database path must not contain null bytes");
// Without the guard, sqlite silently created the truncated path "db3".
expect(existsSync(join(String(dir), "db3"))).toBe(false);
});
});
Loading