What the developer wrote
Authored by Rusty Russell
patch fix.patch
What changed, and why it matters
This patch fixes a timing bug in the bookkeeper plugin of Core Lightning. Previously, setting a currency option before the plugin was fully initialized could start background refresh tasks too early, potentially using uninitialized data. The fix delays starting those refresh tasks until after initialization is complete. It is a correctness fix rather than an obvious security vulnerability, but premature initialization can sometimes lead to crashes or unstable behavior.
Apply the patch. It is a low-risk initialization-order fix. Users relying on the bookkeeper's currency conversion feature should upgrade to avoid possible plugin instability or crashes at startup. No immediate emergency response is warranted unless crashes are observed.
Security signals we found
use of uninitialized or not-yet-initialized plugin state
timing/order-of-initialization bug
potential crash or denial-of-service from premature background task startup
no explicit memory corruption or cryptographic weakness visible in diff
Evidence from the diff
The bookkeeper plugin’s option_currency handler was starting auxiliary command loops (currency_chainmoves_wait / currency_channelmoves_wait) immediately when the –currency option was processed. Because plugin options can be parsed before init() runs, bkpr->accounts and other state might not be ready. The patch moves the refresh startup into a helper start_waiting_for_currency() and calls it from init() if a currency is configured, or from option_currency() only if accounts are already initialized. This prevents starting currency-conversion polling before the plugin is fully set up.
Changed components
plugins/bkpr/bookkeeper.cCore Lightning bookkeeper plugincurrency conversion / fiat-denomination featureInspect captured patch +18 / −5
diff --git a/plugins/bkpr/bookkeeper.c b/plugins/bkpr/bookkeeper.c
index 73283f0a..746c441a 100644
--- a/plugins/bkpr/bookkeeper.c
+++ b/plugins/bkpr/bookkeeper.c
@@ -1728,6 +1728,14 @@ currency_channelmoves_wait(struct command *auxcmd, void *unused)
return send_outreq(req);
}
+/* If we're supposed to do currency conversions, we refresh all the time. */
+static void start_waiting_for_currency(struct bkpr *bkpr, struct command *cmd)
+{
+ bkpr->currency_cmds = aux_command(cmd);
+ currency_chainmoves_wait(bkpr->currency_cmds, NULL);
+ currency_channelmoves_wait(bkpr->currency_cmds, NULL);
+}
+
static const char *init(struct command *init_cmd, const char *b, const jsmntok_t *t)
{
struct plugin *p = init_cmd->plugin;
@@ -1756,6 +1764,10 @@ static const char *init(struct command *init_cmd, const char *b, const jsmntok_t
} else
bkpr->chainmoves_index = 0;
+ /* If we're supposed to do currency conversions, start refresh now. */
+ if (bkpr->currency)
+ start_waiting_for_currency(bkpr, init_cmd);
+
return NULL;
}
@@ -1791,11 +1803,11 @@ static char *option_currency(struct command *cmd,
bkpr->currency = tal_strdup(bkpr, arg);
/* Reset this so we get a new message for new currency */
bkpr->warned_currency_fail = false;
- /* If we're supposed to do currency conversions, we refresh
- * all the time. */
- bkpr->currency_cmds = aux_command(cmd);
- currency_chainmoves_wait(bkpr->currency_cmds, NULL);
- currency_channelmoves_wait(bkpr->currency_cmds, NULL);
+
+ /* Don't do this yet if we're before init! */
+ if (bkpr->accounts)
+ start_waiting_for_currency(bkpr, cmd);
+
return NULL;
}
@@ -1808,6 +1820,7 @@ int main(int argc, char *argv[])
bkpr = tal(NULL, struct bkpr);
bkpr->currency = NULL;
bkpr->currency_rates = tal(bkpr, currencymap_t);
+ bkpr->accounts = NULL;
uintmap_init(bkpr->currency_rates);
memleak_add_helper(bkpr->currency_rates, memleak_scan_currencyrates);
plugin_main(argv, init, take(bkpr), PLUGIN_STATIC, true, NULL,
Why this scored 26/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.