Skip to content

harden(sqlite): add opcode allowlist and buffer size validation to fileControl #27498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 2 commits into from
Closed

harden(sqlite): add opcode allowlist and buffer size validation to fileControl #27498

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
46f4173
harden(sqlite): add opcode allowlist and buffer size validation to fi…
Feb 27, 2026
14ac698
fix(sqlite): add finiteness check for int-sized opcode numeric args
Feb 28, 2026
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
105 changes: 99 additions & 6 deletions src/bun.js/bindings/sqlite/JSSQLStatement.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1756,6 +1756,55 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementCloseStatementFunction, (JSC::JSGlobalObj
return JSValue::encode(jsUndefined());
}

// Returns the minimum required buffer size in bytes for a given SQLITE_FCNTL_*
// opcode, or -1 if the opcode is not allowed. This restricts fileControl to a
// safe subset of opcodes with well-defined argument sizes, preventing misuse
// with undersized buffers or opcodes that expect callback pointers.
static int fcntlMinimumBufferSize(int op)
{
switch (op) {
// Opcodes that use int* (4 bytes):
case SQLITE_FCNTL_LOCKSTATE: // 1
case SQLITE_FCNTL_LAST_ERRNO: // 4
case SQLITE_FCNTL_CHUNK_SIZE: // 6
case SQLITE_FCNTL_PERSIST_WAL: // 10
case SQLITE_FCNTL_POWERSAFE_OVERWRITE: // 13
case SQLITE_FCNTL_WAL_BLOCK: // 24
case SQLITE_FCNTL_LOCK_TIMEOUT: // 34
case SQLITE_FCNTL_DATA_VERSION: // 35
case SQLITE_FCNTL_RESERVE_BYTES: // 38
case SQLITE_FCNTL_EXTERNAL_READER: // 40
return sizeof(int);

// Opcodes that use sqlite3_int64* (8 bytes):
case SQLITE_FCNTL_SIZE_HINT: // 5
case SQLITE_FCNTL_MMAP_SIZE: // 18
case SQLITE_FCNTL_SIZE_LIMIT: // 36
return sizeof(sqlite3_int64);

Comment thread
robobun marked this conversation as resolved.
// Opcodes that take no argument (pArg is unused/NULL is fine):
case SQLITE_FCNTL_SYNC_OMITTED: // 8
case SQLITE_FCNTL_OVERWRITE: // 11
case SQLITE_FCNTL_COMMIT_PHASETWO: // 22
case SQLITE_FCNTL_BEGIN_ATOMIC_WRITE: // 31
case SQLITE_FCNTL_COMMIT_ATOMIC_WRITE: // 32
case SQLITE_FCNTL_ROLLBACK_ATOMIC_WRITE: // 33
case SQLITE_FCNTL_CKPT_DONE: // 37
case SQLITE_FCNTL_CKPT_START: // 39
case SQLITE_FCNTL_RESET_CACHE: // 42
return 0;

// All other opcodes are disallowed. This includes opcodes that:
// - Return internal pointers (FILE_POINTER, VFS_POINTER, JOURNAL_POINTER)
// - Expect callback function pointers (BUSYHANDLER)
// - Are Windows-specific handle operations (WIN32_*)
// - Are internal/extension-specific (PRAGMA, RBU, ZIPVFS, PDB, CKSM_FILE)
// - Are obsolete (GET/SET_LOCKPROXYFILE)
default:
return -1;
}
}

JSC_DEFINE_HOST_FUNCTION(jsSQLStatementFcntlFunction, (JSC::JSGlobalObject * lexicalGlobalObject, JSC::CallFrame* callFrame))
{
auto& vm = JSC::getVM(lexicalGlobalObject);
Expand Down Expand Up @@ -1786,6 +1835,13 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementFcntlFunction, (JSC::JSGlobalObject * lex
int dbIndex = dbNumber.toInt32(lexicalGlobalObject);
int op = opNumber.toInt32(lexicalGlobalObject);

// Validate opcode against allowlist and get minimum buffer size requirement.
int minBufSize = fcntlMinimumBufferSize(op);
if (minBufSize < 0) {
throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Unsupported file control opcode"_s));
return {};
}
Comment thread
robobun marked this conversation as resolved.

if (dbIndex < 0 || dbIndex >= databases().size()) {
throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Invalid database handle"_s));
return {};
Expand All @@ -1804,8 +1860,12 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementFcntlFunction, (JSC::JSGlobalObject * lex
RETURN_IF_EXCEPTION(scope, {});
}

int resultInt = -1;
// Use a stack buffer large enough for any allowed opcode (up to 8 bytes for
// sqlite3_int64). This avoids issues when a number argument is passed but
// the opcode writes more than sizeof(int) bytes.
alignas(sqlite3_int64) char resultBuf[sizeof(sqlite3_int64)] = {};
void* resultPtr = nullptr;

if (resultValue.isObject()) {
if (auto* view = jsDynamicCast<JSC::JSArrayBufferView*>(resultValue.getObject())) {
if (view->isDetached()) {
Expand All @@ -1818,19 +1878,52 @@ JSC_DEFINE_HOST_FUNCTION(jsSQLStatementFcntlFunction, (JSC::JSGlobalObject * lex
throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Expected buffer"_s));
return {};
}

// Validate that the provided buffer is large enough for this opcode.
if (view->byteLength() < static_cast<size_t>(minBufSize)) {
throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, makeString("Buffer too small for this opcode; need at least "_s, minBufSize, " bytes"_s)));
return {};
}
}
} else if (resultValue.isNumber()) {
resultInt = resultValue.toInt32(lexicalGlobalObject);
RETURN_IF_EXCEPTION(scope, {});

resultPtr = &resultInt;
// For opcodes expecting sqlite3_int64, convert to 64-bit so we fill
// the full 8 bytes that sqlite3_file_control will read/write.
if (minBufSize == sizeof(sqlite3_int64)) {
double raw = resultValue.toNumber(lexicalGlobalObject);
RETURN_IF_EXCEPTION(scope, {});
Comment thread
robobun marked this conversation as resolved.
if (!std::isfinite(raw)
|| raw < static_cast<double>(std::numeric_limits<sqlite3_int64>::min())
|| raw > static_cast<double>(std::numeric_limits<sqlite3_int64>::max())) {
throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Expected a finite integer for this opcode"_s));
return {};
}
sqlite3_int64 resultInt64 = static_cast<sqlite3_int64>(raw);
memcpy(resultBuf, &resultInt64, sizeof(resultInt64));
} else {
Comment thread
robobun marked this conversation as resolved.
int resultInt = resultValue.toInt32(lexicalGlobalObject);
Comment thread
robobun marked this conversation as resolved.
RETURN_IF_EXCEPTION(scope, {});
memcpy(resultBuf, &resultInt, sizeof(resultInt));
}
resultPtr = resultBuf;
} else if (resultValue.isNull()) {

// Reject null when the opcode requires writable storage.
if (minBufSize > 0) {
throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "This opcode requires a buffer or value argument"_s));
return {};
}
} else {
throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Expected result to be a number, null or a TypedArray"_s));
return {};
}

// Final safety check: reject nullptr when the opcode requires writable
// storage. This catches cases like passing a plain object (not a TypedArray)
// where the dynamic cast silently fails and resultPtr stays null.
if (resultPtr == nullptr && minBufSize > 0) {
throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "This opcode requires a buffer or value argument"_s));
return {};
}

int statusCode = sqlite3_file_control(db, fileNameStr.isNull() ? nullptr : fileNameStr.data(), op, resultPtr);

if (statusCode == SQLITE_ERROR) {
Expand Down
145 changes: 145 additions & 0 deletions test/js/bun/sqlite/sqlite.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1358,6 +1358,151 @@ it("should close with WAL enabled", () => {
expect(readdirSync(dir).sort()).toEqual(["empty.txt", "my.db"]);
});

describe("fileControl", () => {
it("should work with allowed opcodes and properly sized buffers", () => {
const dir = tempDirWithFiles("sqlite-fcntl-test", { "empty.txt": "" });
const file = path.join(dir, "my.db");
const db = new Database(file);
db.exec("PRAGMA journal_mode = WAL");

// SQLITE_FCNTL_PERSIST_WAL with integer argument (allowed, int-sized)
db.fileControl(constants.SQLITE_FCNTL_PERSIST_WAL, 0);

// SQLITE_FCNTL_DATA_VERSION with a properly sized buffer (4 bytes for int)
const buf = new Uint8Array(4);
db.fileControl(constants.SQLITE_FCNTL_DATA_VERSION, buf);

db.close();
});

it("should reject unsupported opcodes", () => {
const db = new Database(":memory:");

// SQLITE_FCNTL_FILE_POINTER (7) - writes a pointer, not in allowlist
expect(() => db.fileControl(constants.SQLITE_FCNTL_FILE_POINTER, new Uint8Array(8))).toThrow(
"Unsupported file control opcode",
);

// SQLITE_FCNTL_VFS_POINTER (27) - writes a pointer, not in allowlist
expect(() => db.fileControl(constants.SQLITE_FCNTL_VFS_POINTER, new Uint8Array(8))).toThrow(
"Unsupported file control opcode",
);

// SQLITE_FCNTL_JOURNAL_POINTER (28) - writes a pointer, not in allowlist
expect(() => db.fileControl(constants.SQLITE_FCNTL_JOURNAL_POINTER, new Uint8Array(8))).toThrow(
"Unsupported file control opcode",
);

// SQLITE_FCNTL_BUSYHANDLER (15) - expects callback, not in allowlist
expect(() => db.fileControl(constants.SQLITE_FCNTL_BUSYHANDLER, new Uint8Array(16))).toThrow(
"Unsupported file control opcode",
);

// SQLITE_FCNTL_PRAGMA (14) - internal, not in allowlist
expect(() => db.fileControl(constants.SQLITE_FCNTL_PRAGMA, new Uint8Array(24))).toThrow(
"Unsupported file control opcode",
);

// Completely made-up opcode
expect(() => db.fileControl(99999, new Uint8Array(64))).toThrow("Unsupported file control opcode");

db.close();
});

it("should reject undersized buffers for opcodes that write data", () => {
const dir = tempDirWithFiles("sqlite-fcntl-buf-test", { "empty.txt": "" });
const file = path.join(dir, "my.db");
const db = new Database(file);

// SQLITE_FCNTL_PERSIST_WAL needs at least 4 bytes (sizeof int)
expect(() => db.fileControl(constants.SQLITE_FCNTL_PERSIST_WAL, new Uint8Array(1))).toThrow("Buffer too small");
expect(() => db.fileControl(constants.SQLITE_FCNTL_PERSIST_WAL, new Uint8Array(3))).toThrow("Buffer too small");

// Should succeed with exactly 4 bytes
expect(() => db.fileControl(constants.SQLITE_FCNTL_PERSIST_WAL, new Uint8Array(4))).not.toThrow();

// SQLITE_FCNTL_SIZE_LIMIT needs at least 8 bytes (sizeof sqlite3_int64)
expect(() => db.fileControl(constants.SQLITE_FCNTL_SIZE_LIMIT, new Uint8Array(4))).toThrow("Buffer too small");
expect(() => db.fileControl(constants.SQLITE_FCNTL_SIZE_LIMIT, new Uint8Array(7))).toThrow("Buffer too small");

// Should succeed with exactly 8 bytes
expect(() => db.fileControl(constants.SQLITE_FCNTL_SIZE_LIMIT, new Uint8Array(8))).not.toThrow();

db.close();
});

it("should accept null argument for no-arg opcodes", () => {
const dir = tempDirWithFiles("sqlite-fcntl-null-test", { "empty.txt": "" });
const file = path.join(dir, "my.db");
const db = new Database(file);

// SQLITE_FCNTL_RESET_CACHE takes no argument, null is fine
// (May return SQLITE_NOTFOUND on some VFS implementations, which is OK)
const result = db.fileControl(constants.SQLITE_FCNTL_RESET_CACHE, null);
expect(typeof result).toBe("number");

db.close();
});

it("should reject null argument for opcodes that require writable storage", () => {
const dir = tempDirWithFiles("sqlite-fcntl-null-req-test", { "empty.txt": "" });
const file = path.join(dir, "my.db");
const db = new Database(file);

// SQLITE_FCNTL_PERSIST_WAL requires int* storage
expect(() => db.fileControl(constants.SQLITE_FCNTL_PERSIST_WAL, null)).toThrow(
"This opcode requires a buffer or value argument",
);

// SQLITE_FCNTL_DATA_VERSION requires int* storage
expect(() => db.fileControl(constants.SQLITE_FCNTL_DATA_VERSION, null)).toThrow(
"This opcode requires a buffer or value argument",
);

// SQLITE_FCNTL_SIZE_LIMIT requires sqlite3_int64* storage
expect(() => db.fileControl(constants.SQLITE_FCNTL_SIZE_LIMIT, null)).toThrow(
"This opcode requires a buffer or value argument",
);

db.close();
});

it("should handle number arguments correctly for int64 opcodes", () => {
const db = new Database(":memory:");

// SQLITE_FCNTL_SIZE_LIMIT with a number argument should use 64-bit conversion.
// Passing -1 queries the current limit without changing it.
const result = db.fileControl(constants.SQLITE_FCNTL_SIZE_LIMIT, -1);
expect(typeof result).toBe("number");

db.close();
});

it("should reject non-finite numeric arguments for int64 opcodes", () => {
const db = new Database(":memory:");

// NaN and Infinity are not valid for int64 opcodes
expect(() => db.fileControl(constants.SQLITE_FCNTL_SIZE_LIMIT, NaN)).toThrow("Expected a finite integer");
expect(() => db.fileControl(constants.SQLITE_FCNTL_SIZE_LIMIT, Infinity)).toThrow("Expected a finite integer");
expect(() => db.fileControl(constants.SQLITE_FCNTL_SIZE_LIMIT, -Infinity)).toThrow("Expected a finite integer");

db.close();
});

it("should reject plain objects as arguments", () => {
const db = new Database(":memory:");

// A plain object is not a TypedArray - should be treated as a non-matching
// object (resultPtr stays null) and rejected for opcodes needing storage.
// For int-sized opcodes:
expect(() => db.fileControl(constants.SQLITE_FCNTL_PERSIST_WAL, {})).toThrow();
// For int64-sized opcodes:
expect(() => db.fileControl(constants.SQLITE_FCNTL_SIZE_LIMIT, {})).toThrow();

db.close();
});
});
Comment thread
robobun marked this conversation as resolved.

it("close(true) should throw an error if the database is in use", () => {
const db = new Database(":memory:");
db.exec("CREATE TABLE foo (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT)");
Expand Down