plugins/bcli: use -stdin to feed arguments, in case we have a giant tx.
What changed, and why it matters
This commit fixes a bug in Core Lightning's bitcoin-cli plugin (bcli) where very large transactions could make the plugin crash with an 'Argument list too long' error. The fix passes large arguments through standard input instead of the command line, which avoids operating system limits on command-line length. It is a reliability fix rather than a security vulnerability that can be exploited by an attacker.
Treat as a routine reliability fix. No urgent security response is required. Reviewers may want to verify that stdin argument ordering matches bitcoin-cli's expectations and that no secrets are logged by args_string.
Security signals we found
Fixes a denial-of-service-like failure condition (plugin crash) triggered by large transaction data
Uses -stdin to avoid command-line length limits and potential argument exposure
Does not change authentication or authorization logic
No memory safety fixes, cryptographic changes, or input validation changes observed
Evidence from the diff
The bcli plugin previously constructed a bitcoin-cli command line with all RPC arguments, which could exceed ARG_MAX when sending a giant transaction. The patch adds a stdinargs array and uses bitcoin-cli’s -stdin flag (available since Bitcoin Core 0.13) to feed RPC arguments via stdin. RPC password handling is adjusted to include a newline before the stdin arguments. Tests are updated to expect the new -stdin flag and to remove the xfail marker from the giant-transaction test.
Changed components
plugins/bcli.ctests/test_misc.pytests/test_plugin.pyInspect captured patch +34 / −16
diff --git a/plugins/bcli.c b/plugins/bcli.c
index 5203022f..7646b348 100644
--- a/plugins/bcli.c
+++ b/plugins/bcli.c
@@ -76,6 +76,7 @@ struct bitcoin_cli {
int *exitstatus;
pid_t pid;
const char **args;
+ const char **stdinargs;
struct timeabs start;
enum bitcoind_prio prio;
char *output;
@@ -95,7 +96,8 @@ static void add_arg(const char ***args, const char *arg TAKES)
tal_arr_expand(args, arg);
}
-static const char **gather_argsv(const tal_t *ctx, const char *cmd, va_list ap)
+/* If stdinargs is non-NULL, that is where we put additional args */
+static const char **gather_argsv(const tal_t *ctx, const char ***stdinargs, const char *cmd, va_list ap)
{
const char **args = tal_arr(ctx, const char *, 1);
const char *arg;
@@ -128,23 +130,30 @@ static const char **gather_argsv(const tal_t *ctx, const char *cmd, va_list ap)
// `-rpcpassword` argument - secrets in arguments can leak when listing
// system processes.
add_arg(&args, "-stdinrpcpass");
+ /* To avoid giant command lines, we use -stdin (avail since bitcoin 0.13) */
+ if (stdinargs)
+ add_arg(&args, "-stdin");
add_arg(&args, cmd);
- while ((arg = va_arg(ap, char *)) != NULL)
- add_arg(&args, arg);
+ while ((arg = va_arg(ap, char *)) != NULL) {
+ if (stdinargs)
+ add_arg(stdinargs, arg);
+ else
+ add_arg(&args, arg);
+ }
add_arg(&args, NULL);
return args;
}
static LAST_ARG_NULL const char **
-gather_args(const tal_t *ctx, const char *cmd, ...)
+gather_args(const tal_t *ctx, const char ***stdinargs, const char *cmd, ...)
{
va_list ap;
const char **ret;
va_start(ap, cmd);
- ret = gather_argsv(ctx, cmd, ap);
+ ret = gather_argsv(ctx, stdinargs, cmd, ap);
va_end(ap);
return ret;
@@ -170,7 +179,7 @@ static struct io_plan *output_init(struct io_conn *conn, struct bitcoin_cli *bcl
static void next_bcli(enum bitcoind_prio prio);
/* For printing: simple string of args (no secrets!) */
-static char *args_string(const tal_t *ctx, const char **args)
+static char *args_string(const tal_t *ctx, const char **args, const char **stdinargs)
{
size_t i;
char *ret = tal_strdup(ctx, args[0]);
@@ -185,12 +194,16 @@ static char *args_string(const tal_t *ctx, const char **args)
ret = tal_strcat(ctx, take(ret), args[i]);
}
}
+ for (i = 0; i < tal_count(stdinargs); i++) {
+ ret = tal_strcat(ctx, take(ret), " ");
+ ret = tal_strcat(ctx, take(ret), stdinargs[i]);
+ }
return ret;
}
static char *bcli_args(const tal_t *ctx, struct bitcoin_cli *bcli)
{
- return args_string(ctx, bcli->args);
+ return args_string(ctx, bcli->args, bcli->stdinargs);
}
/* Only set as destructor once bcli is in current. */
@@ -313,9 +326,14 @@ static void next_bcli(enum bitcoind_prio prio)
bcli->args[0], strerror(errno));
- if (bitcoind->rpcpass)
+ if (bitcoind->rpcpass) {
write_all(in, bitcoind->rpcpass, strlen(bitcoind->rpcpass));
-
+ write_all(in, "\n", strlen("\n"));
+ }
+ for (size_t i = 0; i < tal_count(bcli->stdinargs); i++) {
+ write_all(in, bcli->stdinargs[i], strlen(bcli->stdinargs[i]));
+ write_all(in, "\n", strlen("\n"));
+ }
close(in);
bcli->start = time_now();
@@ -351,7 +369,8 @@ start_bitcoin_cliv(const tal_t *ctx,
else
bcli->exitstatus = NULL;
- bcli->args = gather_argsv(bcli, method, ap);
+ bcli->stdinargs = tal_arr(bcli, const char *, 0);
+ bcli->args = gather_argsv(bcli, &bcli->stdinargs, method, ap);
bcli->stash = stash;
list_add_tail(&bitcoind->pending[bcli->prio], &bcli->list);
@@ -994,14 +1013,14 @@ static struct command_result *getutxout(struct command *cmd,
static void bitcoind_failure(struct plugin *p, const char *error_message)
{
- const char **cmd = gather_args(bitcoind, "echo", NULL);
+ const char **cmd = gather_args(bitcoind, NULL, "echo", NULL);
plugin_err(p, "\n%s\n\n"
"Make sure you have bitcoind running and that bitcoin-cli"
" is able to connect to bitcoind.\n\n"
"You can verify that your Bitcoin Core installation is"
" ready for use by running:\n\n"
" $ %s 'hello world'\n", error_message,
- args_string(cmd, cmd));
+ args_string(cmd, cmd, NULL));
}
/* Do some sanity checks on bitcoind based on the output of `getnetworkinfo`. */
@@ -1016,7 +1035,7 @@ static void parse_getnetworkinfo_result(struct plugin *p, const char *buf)
if (!result)
plugin_err(p, "Invalid response to '%s': '%s'. Can not "
"continue without proceeding to sanity checks.",
- args_string(tmpctx, gather_args(bitcoind, "getnetworkinfo", NULL)),
+ args_string(tmpctx, gather_args(bitcoind, NULL, "getnetworkinfo", NULL), NULL),
buf);
/* Check that we have a fully-featured `estimatesmartfee`. */
@@ -1047,7 +1066,7 @@ static void wait_and_check_bitcoind(struct plugin *p)
int in, from, status;
pid_t child;
const char **cmd = gather_args(
- bitcoind, "-rpcwait", "-rpcwaittimeout=30", "getnetworkinfo", NULL);
+ bitcoind, NULL, "-rpcwait", "-rpcwaittimeout=30", "getnetworkinfo", NULL);
char *output = NULL;
child = pipecmdarr(&in, &from, &from, cast_const2(char **, cmd));
diff --git a/tests/test_misc.py b/tests/test_misc.py
index b97ba108..97e6ea73 100644
--- a/tests/test_misc.py
+++ b/tests/test_misc.py
@@ -2186,7 +2186,7 @@ def test_bitcoind_fail_first(node_factory, bitcoind):
# first.
timeout = 5 if 5 < TIMEOUT // 3 else TIMEOUT // 3
l1 = node_factory.get_node(start=False,
- broken_log=r'plugin-bcli: .*(-stdinrpcpass getblockhash 100 exited 1 \(after [0-9]* other errors\)|we have been retrying command for)',
+ broken_log=r'plugin-bcli: .*(-stdinrpcpass -stdin getblockhash 100 exited 1 \(after [0-9]* other errors\)|we have been retrying command for)',
may_fail=True,
options={'bitcoin-retry-timeout': timeout})
diff --git a/tests/test_plugin.py b/tests/test_plugin.py
index 314e4369..0a77012d 100644
--- a/tests/test_plugin.py
+++ b/tests/test_plugin.py
@@ -1875,7 +1875,6 @@ def test_bitcoin_backend(node_factory, bitcoind):
" bitcoind")
-@pytest.mark.xfail(strict=True)
def test_bitcoin_backend_gianttx(node_factory, bitcoind):
"""Test that a giant tx doesn't crash bcli"""
l1 = node_factory.get_node(start=False)
Why this scored 33/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.