lightningd: add `get_bitcoin_result` for bcli response handling
What the developer wrote
Authored by dovgopoly
lightningd: add `get_bitcoin_result` for bcli response handling
Add `get_bitcoin_result` function that checks bcli plugin responses for errors and returns the result token. Previously, callbacks only detected errors when result parsing failed, ignoring the explicit error field from the plugin. Now we extract the actual error message from bcli, providing clearer reasoning when the plugin returns an error response.
What changed, and why it matters
This commit refactors how Core Lightning handles replies from its internal Bitcoin plugin (bcli). Previously, if the plugin reported an error, the main daemon might try to read a non-existent 'result' field and produce a confusing 'bad result' message. The new helper explicitly checks for an 'error' field first and extracts the real error message. It also makes several callbacks parse the 'result' object directly instead of expecting it to be wrapped in a top-level '{result:{...}}' structure. This is primarily a robustness/logging improvement; it does not by itself fix a known exploitable vulnerability, but it removes a class of error-handling gaps that could hide or misreport plugin failures.
Treat as a hardening/robustness improvement. Include in normal review and testing; verify that all bcli methods return responses compatible with the new '{result:{...}}' expectation and that error objects include a 'message' field. No emergency action is indicated by the diff alone.
Security signals we found
New centralized error-field handling for Bitcoin plugin responses
Callbacks now fail fast on explicit 'error' objects from bcli
Reduced risk of silently ignoring or misreporting plugin errors
Parsing scope narrowed from full response to 'result' token
Evidence from the diff
The change introduces get_bitcoin_result() in lightningd/bitcoind.c. The helper: (1) looks for an ‘error’ member in the bcli JSON response and, if present, calls bitcoin_plugin_error() with the embedded ‘message’ (or the whole error object), then (2) requires a ‘result’ member and returns its token. Existing callbacks (estimatefees, sendrawtx, getrawblockbyheight, getchaininfo, getutxout) are updated to call this helper and then parse inside the returned result token. Previously, callbacks such as sendrawtx used json_scan on the full response with patterns like ‘{result:{success:%}}’; now they scan resulttok with ‘{success:%}’. The commit improves error propagation and diagnostic clarity when the Bitcoin plugin returns JSON-RPC error objects. It is a defensive refactor rather than a patch for a specific CVE.
Changed components
lightningd/bitcoind.cbcli plugin response handlingestimatefees_callbacksendrawtx_callbackgetrawblockbyheight_callbackgetchaininfo_callbackgetutxout_callbackInspect captured patch +56 / −19
diff --git a/lightningd/bitcoind.c b/lightningd/bitcoind.c
index 58b7b092..c1f461f7 100644
--- a/lightningd/bitcoind.c
+++ b/lightningd/bitcoind.c
@@ -112,6 +112,35 @@ bitcoin_plugin_error(struct bitcoind *bitcoind, const char *buf,
toks->end - toks->start, buf + toks->start);
}
+/* Check response for error (fatal if found) and return result token (fatal if not found). */
+static const jsmntok_t *get_bitcoin_result(struct bitcoind *bitcoind,
+ const char *buf,
+ const jsmntok_t *toks,
+ const char *method)
+{
+ const jsmntok_t *err_tok, *msg_tok, *result_tok;
+
+ err_tok = json_get_member(buf, toks, "error");
+ if (err_tok) {
+ msg_tok = json_get_member(buf, err_tok, "message");
+ if (msg_tok)
+ bitcoin_plugin_error(bitcoind, buf, toks, method,
+ "%.*s", json_tok_full_len(msg_tok),
+ json_tok_full(buf, msg_tok));
+ else
+ bitcoin_plugin_error(bitcoind, buf, toks, method,
+ "%.*s", json_tok_full_len(err_tok),
+ json_tok_full(buf, err_tok));
+ }
+
+ result_tok = json_get_member(buf, toks, "result");
+ if (!result_tok)
+ bitcoin_plugin_error(bitcoind, buf, toks, method,
+ "missing 'result' field");
+
+ return result_tok;
+}
+
/* Send a request to the Bitcoin plugin which registered that method,
* if it's still alive. */
static void bitcoin_plugin_send(struct bitcoind *bitcoind,
@@ -227,11 +256,7 @@ static void estimatefees_callback(const char *buf, const jsmntok_t *toks,
struct feerate_est *feerates;
u32 floor;
- resulttok = json_get_member(buf, toks, "result");
- if (!resulttok)
- bitcoin_plugin_error(call->bitcoind, buf, toks,
- "estimatefees",
- "bad 'result' field");
+ resulttok = get_bitcoin_result(call->bitcoind, buf, toks, "estimatefees");
/* Modern style has floor. */
floortok = json_get_member(buf, resulttok, "feerate_floor");
@@ -315,21 +340,24 @@ static void sendrawtx_callback(const char *buf, const jsmntok_t *toks,
const jsmntok_t *idtok,
struct sendrawtx_call *call)
{
+ const jsmntok_t *resulttok;
const char *err;
const char *errmsg = NULL;
bool success = false;
- err = json_scan(tmpctx, buf, toks, "{result:{success:%}}",
+ resulttok = get_bitcoin_result(call->bitcoind, buf, toks, "sendrawtransaction");
+
+ err = json_scan(tmpctx, buf, resulttok, "{success:%}",
JSON_SCAN(json_to_bool, &success));
if (err) {
- bitcoin_plugin_error(call->bitcoind, buf, toks,
+ bitcoin_plugin_error(call->bitcoind, buf, resulttok,
"sendrawtransaction",
"bad 'result' field: %s", err);
} else if (!success) {
- err = json_scan(tmpctx, buf, toks, "{result:{errmsg:%}}",
+ err = json_scan(tmpctx, buf, resulttok, "{errmsg:%}",
JSON_SCAN_TAL(tmpctx, json_strdup, &errmsg));
if (err)
- bitcoin_plugin_error(call->bitcoind, buf, toks,
+ bitcoin_plugin_error(call->bitcoind, buf, resulttok,
"sendrawtransaction",
"bad 'errmsg' field: %s",
err);
@@ -394,6 +422,7 @@ getrawblockbyheight_callback(const char *buf, const jsmntok_t *toks,
const jsmntok_t *idtok,
struct getrawblockbyheight_call *call)
{
+ const jsmntok_t *resulttok;
const char *block_str, *err;
struct bitcoin_blkid blkid;
struct bitcoin_block *blk;
@@ -401,6 +430,8 @@ getrawblockbyheight_callback(const char *buf, const jsmntok_t *toks,
trace_span_resume(call);
trace_span_end(call);
+ resulttok = get_bitcoin_result(call->bitcoind, buf, toks, "getrawblockbyheight");
+
/* Callback may free parent of call, so steal onto context to
* free if it doesn't */
ctx = tal(NULL, char);
@@ -408,24 +439,24 @@ getrawblockbyheight_callback(const char *buf, const jsmntok_t *toks,
/* If block hash is `null`, this means not found! Call the callback
* with NULL values. */
- err = json_scan(tmpctx, buf, toks, "{result:{blockhash:null}}");
+ err = json_scan(tmpctx, buf, resulttok, "{blockhash:null}");
if (!err) {
call->cb(call->bitcoind, call->height, NULL, NULL, call->cb_arg);
goto clean;
}
- err = json_scan(tmpctx, buf, toks, "{result:{blockhash:%,block:%}}",
+ err = json_scan(tmpctx, buf, resulttok, "{blockhash:%,block:%}",
JSON_SCAN(json_to_sha256, &blkid.shad.sha),
JSON_SCAN_TAL(tmpctx, json_strdup, &block_str));
if (err)
- bitcoin_plugin_error(call->bitcoind, buf, toks,
+ bitcoin_plugin_error(call->bitcoind, buf, resulttok,
"getrawblockbyheight",
"bad 'result' field: %s", err);
blk = bitcoin_block_from_hex(tmpctx, chainparams, block_str,
strlen(block_str));
if (!blk)
- bitcoin_plugin_error(call->bitcoind, buf, toks,
+ bitcoin_plugin_error(call->bitcoind, buf, resulttok,
"getrawblockbyheight",
"bad block");
@@ -494,18 +525,21 @@ static void getchaininfo_callback(const char *buf, const jsmntok_t *toks,
const jsmntok_t *idtok,
struct getchaininfo_call *call)
{
+ const jsmntok_t *resulttok;
const char *err, *chain;
u32 headers, blocks;
bool ibd;
- err = json_scan(tmpctx, buf, toks,
- "{result:{chain:%,headercount:%,blockcount:%,ibd:%}}",
+ resulttok = get_bitcoin_result(call->bitcoind, buf, toks, "getchaininfo");
+
+ err = json_scan(tmpctx, buf, resulttok,
+ "{chain:%,headercount:%,blockcount:%,ibd:%}",
JSON_SCAN_TAL(tmpctx, json_strdup, &chain),
JSON_SCAN(json_to_number, &headers),
JSON_SCAN(json_to_number, &blocks),
JSON_SCAN(json_to_bool, &ibd));
if (err)
- bitcoin_plugin_error(call->bitcoind, buf, toks, "getchaininfo",
+ bitcoin_plugin_error(call->bitcoind, buf, resulttok, "getchaininfo",
"bad 'result' field: %s", err);
call->cb(call->bitcoind, chain, headers, blocks, ibd,
@@ -565,24 +599,27 @@ static void getutxout_callback(const char *buf, const jsmntok_t *toks,
const jsmntok_t *idtok,
struct getutxout_call *call)
{
+ const jsmntok_t *resulttok;
const char *err;
struct bitcoin_tx_output txout;
/* Whatever happens, we want to free this. */
tal_steal(tmpctx, call);
- err = json_scan(tmpctx, buf, toks, "{result:{script:null}}");
+ resulttok = get_bitcoin_result(call->bitcoind, buf, toks, "getutxout");
+
+ err = json_scan(tmpctx, buf, resulttok, "{script:null}");
if (!err) {
call->cb(call->bitcoind, NULL, call->cb_arg);
return;
}
- err = json_scan(tmpctx, buf, toks, "{result:{script:%,amount:%}}",
+ err = json_scan(tmpctx, buf, resulttok, "{script:%,amount:%}",
JSON_SCAN_TAL(tmpctx, json_tok_bin_from_hex,
&txout.script),
JSON_SCAN(json_to_sat, &txout.amount));
if (err)
- bitcoin_plugin_error(call->bitcoind, buf, toks, "getutxout",
+ bitcoin_plugin_error(call->bitcoind, buf, resulttok, "getutxout",
"bad 'result' field: %s", err);
call->cb(call->bitcoind, &txout, call->cb_arg);
Why this scored 23/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.