Skip to content

Commit

Permalink
db: unregister sqlite3 trace callback also in error case
Browse files Browse the repository at this point in the history
For sqlite3 versions < 3.14 (i.e. HAVE_SQLITE3_EXPANDED_SQL is not set),
tracing is used to dump statements. The function db_sqlite3_exec()
registers a tracing callback in the beginning and unregisters it at the
end to "avoid it accessing the potentially stale pointer to stmt".
However, the unregistering so far only happened in the success case,
i.e. if the prepare or step calls failed, the callback was still set!

Running the test wallet/test/db-run with sqlite 3.11 leads to a
segmentation fault in the last call to db_commit_transaction():
the tested transaction contains an invalid statement and the (still
registered) trace callback is triggered then by sqlite3_exec() in
db_sqlite3_commit_tx(), leading to a segfault in db_changes_add()
(according to gdb), where it tries to access "stmt->query->readonly".

Changelog-None
  • Loading branch information
theStack authored and rustyrussell committed May 7, 2020
1 parent 046b402 commit 3a881d9
Showing 1 changed file with 11 additions and 4 deletions.
15 changes: 11 additions & 4 deletions wallet/db_sqlite3.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ static bool db_sqlite3_query(struct db_stmt *stmt)
static bool db_sqlite3_exec(struct db_stmt *stmt)
{
int err;
bool success;
#if !HAVE_SQLITE3_EXPANDED_SQL
/* Register the tracing function if we don't have an explicit way of
* expanding the statement. */
Expand All @@ -108,14 +109,16 @@ static bool db_sqlite3_exec(struct db_stmt *stmt)

if (!db_sqlite3_query(stmt)) {
/* If the prepare step caused an error we hand it up. */
return false;
success = false;
goto done;
}

err = sqlite3_step(stmt->inner_stmt);
if (err != SQLITE_DONE) {
tal_free(stmt->error);
stmt->error = db_sqlite3_fmt_error(stmt);
return false;
success = false;
goto done;
}

#if HAVE_SQLITE3_EXPANDED_SQL
Expand All @@ -124,13 +127,17 @@ static bool db_sqlite3_exec(struct db_stmt *stmt)
expanded_sql = sqlite3_expanded_sql(stmt->inner_stmt);
db_changes_add(stmt, expanded_sql);
sqlite3_free(expanded_sql);
#else
#endif
success = true;

done:
#if !HAVE_SQLITE3_EXPANDED_SQL
/* Unregister the trace callback to avoid it accessing the potentially
* stale pointer to stmt */
sqlite3_trace(stmt->db->conn, NULL, NULL);
#endif

return true;
return success;
}

static bool db_sqlite3_step(struct db_stmt *stmt)
Expand Down

0 comments on commit 3a881d9

Please sign in to comment.