AI-generated analysisPublished automatically and not human-verified. Validated context appears in community notes below.
← Watch feed
Informational 14 Bitcoin

bkpr: convert all the local db sql queries into calls to sql plugin.

Public commit record

What the developer wrote

Authored by Rusty Russell

85/100 · Strong
bkpr: convert all the local db sql queries into calls to sql plugin.

With some help (and hinderance!) from ChatGPT: the field names
differ slightly from our internal db.

The particilar wrinkle is that we have to restrict all queries to
limit them to entries we've seen already. Our code expects this (we
used to only enter it into the db when we processed it), and it would
otherwise be confusing if a sql query returned inconsistent results
because an event occurred while bookkeeper was processing.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode
The short version

What changed, and why it matters

This commit rewrites the bookkeeper (bkpr) plugin so that most of its internal SQLite queries are sent through a new SQL plugin instead of being executed directly. The change is described as a refactoring to use a shared SQL backend. There is no claim in the commit that this fixes a security bug, and the diff does not show an obvious vulnerability. However, because the new queries are built by concatenating strings with user-influenced values (account names, transaction IDs, payment hashes) there is a small risk of SQL injection if the helper functions do not escape those values correctly. The commit also removes a duplicate-check when logging chain events, which could in principle allow duplicate records, but that is a data-integrity issue rather than a direct security flaw.

Recommended action

Review the new sql plugin helpers (sql_string, fmt_bitcoin_txid, fmt_sha256, fmt_bitcoin_outpoint, json_to_outpoint, json_to_msat, chain_events_from_sql, channel_events_from_sql) to confirm they use proper escaping or parameterized queries. Run static analysis and targeted tests for SQL injection with malicious account names or crafted txids/payment hashes. Verify that removing the duplicate check in log_chain_event does not allow duplicate chain_events to be inserted.

Security signals we found

01

Large-scale SQL query rewrite moving from parameterized prepared statements to dynamically built SQL strings

02

User-influenced values embedded into SQL (account_id, txid, payment_hash, outpoint)

03

Removal of in-code duplicate detection for chain event logging

04

No vendor statement that this is a security fix

Risk score

Why this scored 14/100

Our methodology →
Potential impact 2/30
Exploitability 2/25
Stealth signal 1/15
Affected reach 3/15
Confidence 4/10
Evidence quality 2/5
Human-validated context

Community notes

Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.

No validated notes yet.

The AI analysis stands alone for now. Submit a note if you can add evidence or important context.