wallet: store m_additional_flags in SQLiteDatabase to fix reopen path
What the developer wrote
Authored by Pablo Martin
wallet: store m_additional_flags in SQLiteDatabase to fix reopen path
SQLiteDatabase::Open() (the public override) always reopens the database
with no additional flags. If SQLiteBatch::Close() triggers the
force_conn_refresh path (TxnAbort failed), it calls Open() which drops
the original additional_flags, causing in-memory databases to be reopened
as on-disk instead.
Store additional_flags as a member and use it in Open() so the reconnect
preserves the original flags. For in-memory databases, connection recovery
makes no sense as all data would be lost; both the force_conn_refresh path
and the public Open() now throw instead.
Co-authored-by: Sjors Provoost <sjors@sprovoost.nl>
What changed, and why it matters
This patch fixes a bug in Bitcoin Core's SQLite wallet code where an in-memory test wallet could accidentally be reopened as a real on-disk database, or be lost entirely during error recovery. The fix stores the original database flags so reopening preserves them, and explicitly throws an error instead of trying to recover a connection to an in-memory database (which would lose all data). It appears to be a correctness/reliability fix rather than an active security vulnerability in normal production use.
Treat as a routine bugfix/correctness patch. Reviewers should confirm that in-memory wallet tests still behave correctly and that no production path relies on reopening in-memory databases. No urgent security response is indicated by the commit materials alone.
Security signals we found
Data-integrity bug in wallet database reopen path
In-memory database could be silently reopened with different flags
Connection recovery path could discard wallet state for in-memory DBs
No explicit security framing in commit message or diff
Evidence from the diff
SQLiteDatabase::Open() previously re-opened with hardcoded zero additional_flags, so if SQLiteBatch::Close() took the force_conn_refresh path it would call Open() and drop flags such as SQLITE_OPEN_MEMORY. The patch stores m_additional_flags as a member, uses it in Open(), and throws std::runtime_error for in-memory databases in both the public Open() and the recovery path. This prevents an in-memory database from being silently converted to on-disk or being recovered (which would be meaningless because in-memory data is lost when the connection closes).
Changed components
src/wallet/sqlite.cppsrc/wallet/sqlite.hSQLiteDatabaseSQLiteBatch::CloseInspect captured patch +13 / −3
diff --git a/src/wallet/sqlite.cpp b/src/wallet/sqlite.cpp
index 8d39fbae..28123ecf 100644
--- a/src/wallet/sqlite.cpp
+++ b/src/wallet/sqlite.cpp
@@ -116,7 +116,7 @@ SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_pa
{}
SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options, int additional_flags)
- : WalletDatabase(), m_dir_path(dir_path), m_file_path(fs::PathToString(file_path)), m_write_semaphore(1), m_use_unsafe_sync(options.use_unsafe_sync)
+ : WalletDatabase(), m_dir_path(dir_path), m_file_path(fs::PathToString(file_path)), m_additional_flags(additional_flags), m_write_semaphore(1), m_use_unsafe_sync(options.use_unsafe_sync)
{
{
LOCK(g_sqlite_mutex);
@@ -139,7 +139,7 @@ SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_pa
}
try {
- Open(additional_flags);
+ Open(m_additional_flags);
} catch (const std::runtime_error&) {
// If open fails, cleanup this object and rethrow the exception
Cleanup();
@@ -247,7 +247,10 @@ bool SQLiteDatabase::Verify(bilingual_str& error)
void SQLiteDatabase::Open()
{
- Open(/*additional_flags*/0);
+ if (m_additional_flags & SQLITE_OPEN_MEMORY) {
+ throw std::runtime_error("SQLiteDatabase: Cannot reopen an in-memory database");
+ }
+ Open(m_additional_flags);
}
void SQLiteDatabase::Open(int additional_flags)
@@ -448,6 +451,9 @@ void SQLiteBatch::Close()
}
if (force_conn_refresh) {
+ if (m_database.m_additional_flags & SQLITE_OPEN_MEMORY) {
+ throw std::runtime_error("SQLiteDatabase: Cannot recover in-memory database connection");
+ }
m_database.Close();
try {
m_database.Open();
diff --git a/src/wallet/sqlite.h b/src/wallet/sqlite.h
index fb0fa39c..3a35bb24 100644
--- a/src/wallet/sqlite.h
+++ b/src/wallet/sqlite.h
@@ -103,10 +103,14 @@ public:
class SQLiteDatabase : public WalletDatabase
{
private:
+ friend class SQLiteBatch;
+
const fs::path m_dir_path;
const std::string m_file_path;
+ const int m_additional_flags;
+
/**
* This mutex protects SQLite initialization and shutdown.
* sqlite3_config() and sqlite3_shutdown() are not thread-safe (sqlite3_initialize() is).
Why this scored 31/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.