From 9e912ba84adb0ea1c628a40ee043b8aded6bec0f Mon Sep 17 00:00:00 2001 From: Martin Leitner-Ankerl Date: Thu, 23 Jul 2026 16:20:03 +0200 Subject: [PATCH] dbfile: fix use-after-free and leak when recreating a rejected hashfile When an existing hashfile fails dbfile_check() (a schema version or hash-type mismatch), dbfile_prepare() takes the "Recreating hashfile" path: it sqlite3_close()s the handle, unlinks the file, reopens a fresh one, and recurses. But the handle was passed by value, so the reopened handle only ever lived in dbfile_prepare()'s local variable. Its caller, __dbfile_open_handle(), kept - and returned - a pointer to the *closed* handle. __dbfile_open_handle()'s caller then ran sqlite3_prepare_v2() on that freed handle (use-after-free; SQLITE_MISUSE / "Database error 21"), and later sqlite3_close()d it again, while the working handle opened during recreation was leaked (~160 KB, "definitely lost" under valgrind). Pass the handle by reference (sqlite3 **) so the reopened handle propagates back to the caller, and bail out cleanly if the reopen fails (the previous code would also have recursed into dbfile_prepare(NULL)). Normal runs happened to read the freed-but-intact memory and passed; valgrind exposes it whenever a hashfile is rebuilt - e.g. opening an older-schema hashfile with a newer duperemove. Co-authored-by: Claude Opus 4.8 --- dbfile.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/dbfile.c b/dbfile.c index cf8deb3917be..cb165f517afc 100644 --- a/dbfile.c +++ b/dbfile.c @@ -257,8 +257,9 @@ static int dbfile_set_modes(sqlite3 *db) return ret; } -static int dbfile_prepare(sqlite3 *db) +static int dbfile_prepare(sqlite3 **db_p) { + sqlite3 *db = *db_p; struct dbfile_config cfg; int ret; char dbpath[PATH_MAX + 1]; @@ -306,8 +307,18 @@ static int dbfile_prepare(sqlite3 *db) return ret; } + /* + * Hand the freshly-opened handle back to the caller: + * dbfile_prepare took *db_p by reference precisely so this + * replacement propagates. The old handle was just closed above; + * returning it (as the by-value version did) left the caller + * using freed memory and leaking this new one. + */ db = __dbfile_open_handle(dbpath, false); - return dbfile_prepare(db); + *db_p = db; + if (!db) + return -1; + return dbfile_prepare(db_p); } /* May store the default config, if fields were missing @@ -359,7 +370,7 @@ static sqlite3 *__dbfile_open_handle(char *filename, bool force_create) return NULL; } - ret = dbfile_prepare(db); + ret = dbfile_prepare(&db); if (ret) { sqlite3_close(db); return NULL;