fix(cpp): bind native ATTACH/DETACH parameters and use SQLCipher key API

android/CMakeLists.txt

@@ -3,14 +3,20 @@ cmake_minimum_required(VERSION 3.9.0)
 
 set (PACKAGE_NAME "op-sqlite")
 
-include_directories(
-  ../cpp
-)
-
 if (USE_SQLCIPHER)
+  # Put ../cpp/sqlcipher ahead of ../cpp on the include path so the SQLCipher
+  # header (which exposes sqlite3_key_v2 and friends) wins over the plain
+  # sqlite3.h that also lives in ../cpp. Quoted #include search has its own
+  # rules, but with this ordering both quoted and angle-bracket forms resolve
+  # to the SQLCipher header under USE_SQLCIPHER; without it the SQLCipher
+  # build was empirically picking up the plain header.
   include_directories(../cpp/sqlcipher)
 endif()
 
+include_directories(
+  ../cpp
+)
+
 if (USE_LIBSQL)
   include_directories(src/main/jniLibs/include)
 endif()

cpp/DBHostObject.cpp

@@ -251,6 +251,19 @@ void DBHostObject::create_jsi_functions(jsi::Runtime &rt) {
       secondary_db_path = secondary_db_path + location;
     }
 
+    // Reject zero bytes uniformly: libsql's libsql_bind_string takes a
+    // C-string and would silently truncate at the first zero byte;
+    // SQLite and Turso bind with explicit lengths. Failing loudly across
+    // all backends keeps behaviour consistent.
+    if (secondary_db_name.find('\0') != std::string::npos) {
+      throw std::runtime_error(
+          "[op-sqlite] attach secondaryDbFileName must not contain a zero byte");
+    }
+    if (alias.find('\0') != std::string::npos) {
+      throw std::runtime_error(
+          "[op-sqlite] attach alias must not contain a zero byte");
+    }
+
 #ifdef OP_SQLITE_USE_LIBSQL
     opsqlite_libsql_attach(db, secondary_db_path, secondary_db_name, alias);
 #else
@@ -266,6 +279,10 @@ void DBHostObject::create_jsi_functions(jsi::Runtime &rt) {
     }
 
     std::string alias = args[0].asString(rt).utf8(rt);
+    if (alias.find('\0') != std::string::npos) {
+      throw std::runtime_error(
+          "[op-sqlite] detach alias must not contain a zero byte");
+    }
 #ifdef OP_SQLITE_USE_LIBSQL
     opsqlite_libsql_detach(db, alias);
 #else

cpp/bridge.cpp

@@ -103,7 +103,23 @@
 
 #ifdef OP_SQLITE_USE_SQLCIPHER
   if (!encryption_key.empty()) {
-    opsqlite_execute(db, "PRAGMA key = '" + encryption_key + "'", nullptr);
+    // Use the SQLCipher C API directly instead of `PRAGMA key = '...'`.
+    // Primary reason: removes the SQL-injection shape — if the key string
+    // is ever attacker-influenced, a payload like `'; ATTACH ...; --`
+    // would otherwise execute arbitrary SQL on the freshly-opened
+    // connection. Secondary benefits: the key is passed as a binary
+    // buffer with explicit length so embedded zero bytes in the key are
+    // preserved, and it never enters trace/log surfaces — defense in
+    // depth, not an exploit class on its own.
+    int key_status = sqlite3_key_v2(
+        db, "main", encryption_key.data(),
+        static_cast<int>(encryption_key.size()));
+    if (key_status != SQLITE_OK) {
+      const char *message = sqlite3_errmsg(db);
+      throw std::runtime_error(
+          "[op-sqlite] failed to set encryption key: " +
+          std::string(message != nullptr ? message : "unknown error"));
+    }
   }
 #endif
 
@@ -161,15 +177,17 @@
 void opsqlite_attach(sqlite3 *db, std::string const &doc_path,
                      std::string const &secondary_db_name,
                      std::string const &alias) {
+  // ATTACH/DETACH's filename and schema-name slots both accept parameters
+  // (verified on SQLite 3.50.6, behavior present across the 3.x series).
+  // Binding sidesteps every escaping/identifier-quoting concern.
   auto secondary_db_path = opsqlite_get_db_path(secondary_db_name, doc_path);
-  auto statement = "ATTACH DATABASE '" + secondary_db_path + "' AS " + alias;
-
-  opsqlite_execute(db, statement, nullptr);
+  std::vector<JSVariant> params = {secondary_db_path, alias};
+  opsqlite_execute(db, "ATTACH DATABASE ? AS ?", &params);
 }
 
 void opsqlite_detach(sqlite3 *db, std::string const &alias) {
-  std::string statement = "DETACH DATABASE " + alias;
-  opsqlite_execute(db, statement, nullptr);
+  std::vector<JSVariant> params = {alias};
+  opsqlite_execute(db, "DETACH DATABASE ?", &params);
 }
 
 void opsqlite_remove(sqlite3 *db, std::string const &name,
@@ -310,7 +328,7 @@
   if (isFailed) {
     throw std::runtime_error(
         "[op-sqlite] SQLite code: " + std::to_string(result) +
         " execution error: " + std::string(errorMessage));
   }
 
   int changedRowCount = sqlite3_changes(db);
@@ -624,7 +642,7 @@
   if (isFailed) {
     throw std::runtime_error(
         "[op-sqlite] SQLite error code: " + std::to_string(result) +
         ", description: " + std::string(errorMessage));
   }
 
   int changedRowCount = sqlite3_changes(db);
@@ -756,7 +774,7 @@
   if (isFailed) {
     throw std::runtime_error(
         "[op-sqlite] SQLite error code: " + std::to_string(step) +
         ", description: " + std::string(errorMessage));
   }
 
   int changedRowCount = sqlite3_changes(db);

cpp/libsql/bridge.cpp

@@ -155,14 +155,13 @@ void opsqlite_libsql_attach(DB const &db, std::string const &docPath,
                             std::string const &databaseToAttach,
                             std::string const &alias) {
     std::string dbPath = opsqlite_get_db_path(databaseToAttach, docPath);
-    std::string statement = "ATTACH DATABASE '" + dbPath + "' AS " + alias;
-
-    opsqlite_libsql_execute(db, statement, nullptr);
+    std::vector<JSVariant> params = {dbPath, alias};
+    opsqlite_libsql_execute(db, "ATTACH DATABASE ? AS ?", &params);
 }
 
 void opsqlite_libsql_detach(DB const &db, std::string const &alias) {
-    std::string statement = "DETACH DATABASE " + alias;
-    opsqlite_libsql_execute(db, statement, nullptr);
+    std::vector<JSVariant> params = {alias};
+    opsqlite_libsql_execute(db, "DETACH DATABASE ?", &params);
 }
 
 int32_t opsqlite_libsql_get_reserved_bytes(DB const &db) {

cpp/turso_bridge.cpp

@@ -593,12 +593,13 @@ void opsqlite_attach(sqlite3 *db, std::string const &doc_path,
                      std::string const &secondary_db_name,
                      std::string const &alias) {
   auto secondary_db_path = opsqlite_get_db_path(secondary_db_name, doc_path);
-  auto statement = "ATTACH DATABASE '" + secondary_db_path + "' AS " + alias;
-  opsqlite_execute(db, statement, nullptr);
+  std::vector<JSVariant> params = {secondary_db_path, alias};
+  opsqlite_execute(db, "ATTACH DATABASE ? AS ?", &params);
 }
 
 void opsqlite_detach(sqlite3 *db, std::string const &alias) {
-  opsqlite_execute(db, "DETACH DATABASE " + alias, nullptr);
+  std::vector<JSVariant> params = {alias};
+  opsqlite_execute(db, "DETACH DATABASE ?", &params);
 }
 
 sqlite3_stmt *opsqlite_prepare_statement(sqlite3 *db,

example/src/tests/dbsetup.ts

@@ -270,6 +270,179 @@ it("Can attach/dettach database", () => {
 	db2.delete();
 });
 
+it("Neutralizes SQL injection payload in attach alias", () => {
+	if (isTurso()) {
+		return;
+	}
+	const db = open({
+		name: "attachInjectionTest.sqlite",
+		encryptionKey: "test",
+	});
+	let db2 = open({
+		name: "attachInjectionTest2.sqlite",
+		encryptionKey: "test",
+	});
+	db2.close();
+
+	// Pre-fix, `opsqlite_execute` walked remainingStatement, so the trailing
+	// `ATTACH ... AS pwned` would have run as a second prepared statement.
+	// Post-fix the alias is passed via parameter binding (`ATTACH DATABASE ?
+	// AS ?`), so the whole payload is a single TEXT value used as the
+	// schema-name; neither `pwned` nor `evil` ends up attached. The
+	// `:memory:` target in the payload keeps the proof side-effect-free
+	// across sandboxes in case the pre-fix code path is ever reintroduced.
+	const maliciousAlias = "evil; ATTACH DATABASE ':memory:' AS pwned; --";
+
+	db.attach({
+		secondaryDbFileName: "attachInjectionTest2.sqlite",
+		alias: maliciousAlias,
+	});
+
+	let pwnedAttached = false;
+	try {
+		db.executeSync("SELECT 1 FROM pwned.sqlite_master LIMIT 1;");
+		pwnedAttached = true;
+	} catch {
+		pwnedAttached = false;
+	}
+	expect(pwnedAttached).toBe(false);
+
+	let evilAttached = false;
+	try {
+		db.executeSync("SELECT 1 FROM evil.sqlite_master LIMIT 1;");
+		evilAttached = true;
+	} catch {
+		evilAttached = false;
+	}
+	expect(evilAttached).toBe(false);
+
+	db.detach(maliciousAlias);
+
+	db.delete();
+
+	db2 = open({
+		name: "attachInjectionTest2.sqlite",
+		encryptionKey: "test",
+	});
+	db2.delete();
+});
+
+it("Neutralizes SQL injection payload in attach path", () => {
+	if (isTurso()) {
+		return;
+	}
+	// `opsqlite_get_db_path` just concatenates location + filename, so any
+	// quote in the filename used to escape the surrounding string literal
+	// in `ATTACH DATABASE '...'`. Post-fix the path is passed via parameter
+	// binding, so the embedded quote is just data — no SQL involvement.
+	const quirkyFileName = "attach'Injection.sqlite";
+	const db = open({
+		name: "attachPathHostDb.sqlite",
+		encryptionKey: "test",
+	});
+	const db2 = open({
+		name: quirkyFileName,
+		encryptionKey: "test",
+	});
+	db2.close();
+
+	db.attach({
+		secondaryDbFileName: quirkyFileName,
+		alias: "quirky",
+	});
+	db.executeSync("DROP TABLE IF EXISTS quirky.canary;");
+	db.executeSync(
+		"CREATE TABLE IF NOT EXISTS quirky.canary (id INTEGER PRIMARY KEY);",
+	);
+	db.executeSync("INSERT INTO quirky.canary (id) VALUES (1);");
+	const rows = db.executeSync("SELECT id FROM quirky.canary;").rows;
+	expect(rows[0]!.id).toBe(1);
+
+	db.detach("quirky");
+	db.delete();
+
+	open({ name: quirkyFileName, encryptionKey: "test" }).delete();
+});
+
+it("Detach with injection payload does not execute trailing SQL", () => {
+	if (isTurso()) {
+		return;
+	}
+	const db = open({
+		name: "detachInjectionTest.sqlite",
+		encryptionKey: "test",
+	});
+	const secondary = open({
+		name: "detachInjectionTest2.sqlite",
+		encryptionKey: "test",
+	});
+	secondary.close();
+
+	db.executeSync("DROP TABLE IF EXISTS canary;");
+	db.executeSync("CREATE TABLE canary (id INTEGER PRIMARY KEY);");
+	db.executeSync("INSERT INTO canary (id) VALUES (42);");
+
+	// Attach a *real* schema named `safe` so the leading DETACH actually
+	// resolves pre-fix. The trailing `DROP TABLE canary; --` would then
+	// run as a second prepared statement and the canary row would be gone.
+	// Post-fix the alias is passed via parameter binding, so the whole
+	// payload is the schema-name to detach; nothing matches. The core
+	// proof is that `canary` survives — backends differ on whether a
+	// missing-schema DETACH errors (sqlite/sqlcipher) or returns cleanly
+	// (libsql), so we don't assert on the throw shape.
+	db.attach({
+		secondaryDbFileName: "detachInjectionTest2.sqlite",
+		alias: "safe",
+	});
+
+	try {
+		db.detach("safe; DROP TABLE canary; --");
+	} catch {
+		// Ignored — only the canary check below is load-bearing.
+	}
+
+	const rows = db.executeSync("SELECT id FROM canary;").rows;
+	expect(rows.length).toBe(1);
+	expect(rows[0]!.id).toBe(42);
+
+	// Best-effort cleanup of `safe`. On backends where the malicious
+	// detach above did not throw, libsql may also have detached the
+	// `safe` schema (treating the bound text as a literal alias-name
+	// that didn't match) or left it attached; either way, swallow the
+	// error so cleanup doesn't fail the test.
+	try {
+		db.detach("safe");
+	} catch {
+		// Already detached or never attached — ignore.
+	}
+	db.delete();
+
+	open({ name: "detachInjectionTest2.sqlite", encryptionKey: "test" }).delete();
+});
+
+if (isSQLCipher()) {
+	it("Encryption key with single quote survives a round-trip", () => {
+		// Prior code embedded the key directly into `PRAGMA key = '<key>'`,
+		// so a quote in the key would either error out or silently set a
+		// truncated key. Post-fix the key is set via `sqlite3_key_v2`,
+		// which takes a binary buffer + length — preserving the key
+		// exactly, so reopening with the same key still decrypts.
+		const trickyKey = "p'a''ss\"wrd";
+		const dbName = "pragmaKeyInjectionTest.sqlite";
+
+		let db = open({ name: dbName, encryptionKey: trickyKey });
+		db.executeSync("DROP TABLE IF EXISTS secret;");
+		db.executeSync("CREATE TABLE secret (value TEXT);");
+		db.executeSync("INSERT INTO secret (value) VALUES ('ok');");
+		db.close();
+
+		db = open({ name: dbName, encryptionKey: trickyKey });
+		const rows = db.executeSync("SELECT value FROM secret;").rows;
+		expect(rows[0]!.value).toBe("ok");
+		db.delete();
+	});
+}
+
 it("Can get db path", () => {
 	const db = open({
 		name: "pathTest.sqlite",