From 8f6d3d87bb1eeae55fd2d42e6f8ba182ddb3002f Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 23 Oct 2025 14:22:56 +1030 Subject: [PATCH] plugins: fix %*.s typo. And add a check for new uses creeping in, since it got cut & paste everywhere. This means "this is a valid string, but truncate it to this many characters" vs "%.*s" which means "only read this many characters of string": ``` ['lightningd-3 2025-10-23T02:31:40.890Z **BROKEN** plugin-funder: Plugin marked as important, shutting down lightningd!'] --------------------------- Captured stderr teardown --------------------------- #0 0x557da58ad1dc in printf_common(void*, char const*, __va_list_tag*) asan_interceptors.cpp.o #1 0x557da5aff814 in json_out_addv /home/runner/work/lightning/lightning/ccan/ccan/json_out/json_out.c:239:11 #2 0x557da59740ce in plugin_logv /home/runner/work/lightning/lightning/plugins/libplugin.c:1777:2 #3 0x557da5969b6f in plugin_log /home/runner/work/lightning/lightning/plugins/libplugin.c:1934:2 #4 0x557da595c4f6 in datastore_del_success /home/runner/work/lightning/lightning/plugins/funder.c:161:2 #5 0x557da598b837 in handle_rpc_reply /home/runner/work/lightning/lightning/plugins/libplugin.c:1072:10 #6 0x557da598a4b0 in rpc_conn_read_response /home/runner/work/lightning/lightning/plugins/libplugin.c:1361:3 #7 0x557da5adbea5 in next_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:60:9 #8 0x557da5ae06ff in do_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:422:8 #9 0x557da5adfb58 in io_ready /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:439:10 #10 0x557da5aec2ce in io_loop /home/runner/work/lightning/lightning/ccan/ccan/io/poll.c:455:5 #11 0x557da59757ac in plugin_main /home/runner/work/lightning/lightning/plugins/libplugin.c:2409:3 #12 0x557da594fe23 in main /home/runner/work/lightning/lightning/plugins/funder.c:1723:2 #13 0x7f6572229d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 #14 0x7f6572229e3f in __libc_start_main csu/../csu/libc-start.c:392:3 #15 0x557da588b584 in _start (/home/runner/work/lightning/lightning/plugins/funder+0x10d584) (BuildId: 71ba63ab577fc6fa60573d3e8555f6db7d5c584d) 0x624000009d28 is located 0 bytes to the right of 7208-byte region [0x624000008100,0x624000009d28) allocated by thread T0 here: #0 0x557da590e7f6 in __interceptor_realloc (/home/runner/work/lightning/lightning/plugins/funder+0x1907f6) (BuildId: 71ba63ab577fc6fa60573d3e8555f6db7d5c584d) #1 0x557da5b2149b in tal_resize_ /home/runner/work/lightning/lightning/ccan/ccan/tal/tal.c:755:13 #2 0x557da59f2032 in membuf_tal_resize /home/runner/work/lightning/lightning/common/utils.c:203:2 #3 0x557da5b03934 in membuf_prepare_space_ /home/runner/work/lightning/lightning/ccan/ccan/membuf/membuf.c:45:12 #4 0x557da59d4289 in jsonrpc_io_read_ /home/runner/work/lightning/lightning/common/jsonrpc_io.c:127:2 #5 0x557da598a635 in rpc_conn_read_response /home/runner/work/lightning/lightning/plugins/libplugin.c:1366:9 #6 0x557da5adbea5 in next_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:60:9 #7 0x557da5ae06ff in do_plan /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:422:8 #8 0x557da5adfb58 in io_ready /home/runner/work/lightning/lightning/ccan/ccan/io/io.c:439:10 #9 0x557da5aec2ce in io_loop /home/runner/work/lightning/lightning/ccan/ccan/io/poll.c:455:5 #10 0x557da59757ac in plugin_main /home/runner/work/lightning/lightning/plugins/libplugin.c:2409:3 #11 0x557da594fe23 in main /home/runner/work/lightning/lightning/plugins/funder.c:1723:2 #12 0x7f6572229d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16 SUMMARY: AddressSanitizer: heap-buffer-overflow asan_interceptors.cpp.o in printf_common(void*, char const*, __va_list_tag*) Shadow bytes around the buggy address: 0x0c487fff9350: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9360: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9370: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c487fff9390: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 =>0x0c487fff93a0: 00 00 00 00 00[fa]fa fa fa fa fa fa fa fa fa fa 0x0c487fff93b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff93c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff93d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff93e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c487fff93f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==26122==ABORTING ``` Signed-off-by: Rusty Russell --- Makefile | 5 ++++- plugins/funder.c | 18 +++++++++--------- plugins/spender/multifundchannel.c | 2 +- plugins/spender/openchannel.c | 2 +- plugins/txprepare.c | 4 ++-- 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index 3cee618f9..32293fd73 100644 --- a/Makefile +++ b/Makefile @@ -585,6 +585,9 @@ check-tmpctx: check-discouraged-functions: @if git grep -E "[^a-z_/](fgets|fputs|gets|scanf|sprintf)\(" -- "*.c" "*.h" ":(exclude)ccan/" ":(exclude)contrib/"; then exit 1; fi +check-bad-sprintf: + @if git grep -n "%[*]\.s"; then exit 1; fi + # Don't access amount_msat and amount_sat members directly without a good reason # since it risks overflow. check-amount-access: @@ -609,7 +612,7 @@ check-doc-examples: update-doc-examples git diff --exit-code HEAD # For those without working cppcheck -check-source-no-cppcheck: check-makefile check-source-bolt check-whitespace check-spelling check-python check-includes check-shellcheck check-setup_locale check-tmpctx check-discouraged-functions check-amount-access +check-source-no-cppcheck: check-makefile check-source-bolt check-whitespace check-spelling check-python check-includes check-shellcheck check-setup_locale check-tmpctx check-discouraged-functions check-amount-access check-bad-sprintf check-source: check-source-no-cppcheck diff --git a/plugins/funder.c b/plugins/funder.c index 15ab3f100..2843e5987 100644 --- a/plugins/funder.c +++ b/plugins/funder.c @@ -80,7 +80,7 @@ unreserve_done(struct command *aux_cmd, struct pending_open *open) { plugin_log(open->p, LOG_DBG, - "`unreserveinputs` for channel %s completed. %*.s", + "`unreserveinputs` for channel %s completed. %.*s", fmt_channel_id(tmpctx, &open->channel_id), json_tok_full_len(result), json_tok_full(buf, result)); @@ -159,7 +159,7 @@ datastore_del_success(struct command *cmd, { /* Cool we deleted some stuff */ plugin_log(cmd->plugin, LOG_DBG, - "`datastore` del succeeded: %*.s", + "`datastore` del succeeded: %.*s", json_tok_full_len(result), json_tok_full(buf, result)); @@ -175,7 +175,7 @@ datastore_add_fail(struct command *cmd, { /* Oops, something's broken */ plugin_log(cmd->plugin, LOG_BROKEN, - "%s failed: %*.s", + "%s failed: %.*s", method, json_tok_full_len(error), json_tok_full(buf, error)); @@ -197,7 +197,7 @@ datastore_add_success(struct command *cmd, if (err) plugin_err(cmd->plugin, - "`datastore` payload did not scan. %s: %*.s", + "`datastore` payload did not scan. %s: %.*s", err, json_tok_full_len(result), json_tok_full(buf, result)); @@ -266,7 +266,7 @@ signpsbt_done(struct command *cmd, if (err) plugin_err(cmd->plugin, - "`signpsbt` payload did not scan %s: %*.s", + "`signpsbt` payload did not scan %s: %.*s", err, json_tok_full_len(result), json_tok_full(buf, result)); @@ -594,7 +594,7 @@ listfunds_success(struct command *cmd, outputs_tok = json_get_member(buf, result, "outputs"); if (!outputs_tok) plugin_err(cmd->plugin, - "`listfunds` payload has no outputs token: %*.s", + "`listfunds` payload has no outputs token: %.*s", json_tok_full_len(result), json_tok_full(buf, result)); @@ -624,7 +624,7 @@ listfunds_success(struct command *cmd, JSON_SCAN(json_to_number, &utxo->out.n)); if (err) plugin_err(cmd->plugin, - "`listfunds` payload did not scan. %s: %*.s", + "`listfunds` payload did not scan. %s: %.*s", err, json_tok_full_len(result), json_tok_full(buf, result)); @@ -923,7 +923,7 @@ datastore_list_fail(struct command *cmd, /* Oops, something's broken */ plugin_log(cmd->plugin, LOG_BROKEN, - "`datastore` list failed: %*.s", + "`datastore` list failed: %.*s", json_tok_full_len(error), json_tok_full(buf, error)); @@ -965,7 +965,7 @@ datastore_list_success(struct command *cmd, if (err) plugin_err(cmd->plugin, "`listdatastore` payload did" - " not scan. %s: %*.s", + " not scan. %s: %.*s", err, json_tok_full_len(result), json_tok_full(buf, result)); diff --git a/plugins/spender/multifundchannel.c b/plugins/spender/multifundchannel.c index af425736e..c7223867c 100644 --- a/plugins/spender/multifundchannel.c +++ b/plugins/spender/multifundchannel.c @@ -1450,7 +1450,7 @@ after_getfeerate(struct command *cmd, JSON_SCAN(json_to_number, &feerate)); if (err) mfc_fail(mfc, JSONRPC2_INVALID_PARAMS, - "Unable to parse feerate %s: %*.s", + "Unable to parse feerate %s: %.*s", err, json_tok_full_len(result), json_tok_full(buf, result)); diff --git a/plugins/spender/openchannel.c b/plugins/spender/openchannel.c index 85d15b06f..7d2191620 100644 --- a/plugins/spender/openchannel.c +++ b/plugins/spender/openchannel.c @@ -554,7 +554,7 @@ static struct command_result *json_peer_sigs(struct command *cmd, JSON_SCAN_TAL(cmd, json_to_psbt, &psbt)); if (err) plugin_err(cmd->plugin, - "`openchannel_peer_sigs` did not scan: %s. %*.s", + "`openchannel_peer_sigs` did not scan: %s. %.*s", err, json_tok_full_len(params), json_tok_full(buf, params)); diff --git a/plugins/txprepare.c b/plugins/txprepare.c index d1be784e9..5d97b7e48 100644 --- a/plugins/txprepare.c +++ b/plugins/txprepare.c @@ -522,7 +522,7 @@ static struct command_result *listfunds_done(struct command *cmd, txp->output_total = AMOUNT_SAT(0); if (!outputs_tok) plugin_err(cmd->plugin, - "`listfunds` payload has no outputs token: %*.s", + "`listfunds` payload has no outputs token: %.*s", json_tok_full_len(result), json_tok_full(buf, result)); @@ -547,7 +547,7 @@ static struct command_result *listfunds_done(struct command *cmd, JSON_SCAN(json_to_number, &prev_out.n)); if (err) plugin_err(cmd->plugin, - "`listfunds` payload did not scan. %s: %*.s", + "`listfunds` payload did not scan. %s: %.*s", err, json_tok_full_len(result), json_tok_full(buf, result));