diff --git a/src/bun.js/bindings/sqlite/JSSQLStatement.cpp b/src/bun.js/bindings/sqlite/JSSQLStatement.cpp index a94c9ba7d9c..26de7ab52ae 100644 --- a/src/bun.js/bindings/sqlite/JSSQLStatement.cpp +++ b/src/bun.js/bindings/sqlite/JSSQLStatement.cpp @@ -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); + + // 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); @@ -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 {}; + } + if (dbIndex < 0 || dbIndex >= databases().size()) { throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Invalid database handle"_s)); return {}; @@ -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(resultValue.getObject())) { if (view->isDetached()) { @@ -1818,19 +1878,58 @@ 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(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, {}); + if (!std::isfinite(raw) + || raw < static_cast(std::numeric_limits::min()) + || raw > static_cast(std::numeric_limits::max())) { + throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Expected a finite integer for this opcode"_s)); + return {}; + } + sqlite3_int64 resultInt64 = static_cast(raw); + memcpy(resultBuf, &resultInt64, sizeof(resultInt64)); + } else { + double raw = resultValue.toNumber(lexicalGlobalObject); + RETURN_IF_EXCEPTION(scope, {}); + if (!std::isfinite(raw)) { + throwException(lexicalGlobalObject, scope, createError(lexicalGlobalObject, "Expected a finite integer for this opcode"_s)); + return {}; + } + int resultInt = resultValue.toInt32(lexicalGlobalObject); + 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) { diff --git a/test/js/bun/sqlite/sqlite.test.js b/test/js/bun/sqlite/sqlite.test.js index c5681f4ea00..2f31fa88f30 100644 --- a/test/js/bun/sqlite/sqlite.test.js +++ b/test/js/bun/sqlite/sqlite.test.js @@ -1358,6 +1358,165 @@ 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 non-finite numeric arguments for int-sized opcodes", () => { + const dir = tempDirWithFiles("sqlite-fcntl-nan-int-test", { "empty.txt": "" }); + const file = path.join(dir, "my.db"); + const db = new Database(file); + db.exec("PRAGMA journal_mode = WAL"); + + // NaN and Infinity are not valid for int-sized opcodes either + expect(() => db.fileControl(constants.SQLITE_FCNTL_PERSIST_WAL, NaN)).toThrow("Expected a finite integer"); + expect(() => db.fileControl(constants.SQLITE_FCNTL_PERSIST_WAL, Infinity)).toThrow("Expected a finite integer"); + expect(() => db.fileControl(constants.SQLITE_FCNTL_PERSIST_WAL, -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(); + }); +}); + 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)");