rpc,wallet: prevent user input injecting fake log lines #35833

Problem: Restricted RPC users and callers of createwallet or restorewallet can inject newlines through rejected methods or wallet names, and global log escaping preserves them, making forged lines look like node messages. Wallet names also serve as filesystem paths and appear in UIs, where invisible controls are undesirable. [Comments in #35768](/bitcoin-bitcoin/35768/#issuecomment-5050755259) indicated that we can still reject invalid new names while keeping existing names loadable.

Fix: Sanitize rejected method names on both whitelist paths and reject control characters in names supplied through createwallet, GUI wallet creation, or restorewallet. Authorization still uses the original method string, ordinary wallet loading remains unchanged, and migration rollback restores an existing name with a warning.

<details> <summary>Manual reproducer</summary>

DATADIR="$(mktemp -d /tmp/bitcoin-log-injection.XXXXXX)"
M="$(date -u +%Y-%m-%dT%H:%M:%SZ) ERROR: ConnectTip: ConnectBlock 0000000000000000deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef failed, bad-txns-inputs-missingorspent"
cmake -B build >/dev/null 2>&1 && cmake --build build -j >/dev/null 2>&1
build/bin/bitcoind -regtest -daemonwait -datadir="$DATADIR" -rpcwhitelist=__cookie__:getblock,stop >/dev/null 2>&1
build/bin/bitcoin-cli -regtest -datadir="$DATADIR" $'getblock\n'"$M" >/dev/null 2>&1; killall bitcoind >/dev/null
echo; grep -E 'ConnectTip|not allowed' "$DATADIR/regtest/debug.log"

Before

2026-07-28T23:18:57Z [warning] RPC User __cookie__ not allowed to call method getblock
2026-07-28T23:18:55Z ERROR: ConnectTip: ConnectBlock 0000000000000000deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef failed, bad-txns-inputs-missingorspent

After

2026-07-28T23:19:45Z [warning] RPC User __cookie__ not allowed to call method getblock2026-07-28T23:19:41Z ERROR: ConnectTip: ConnectBlock 0000000000000000deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef failed, bad-txns-inputs-missingorspent

</details>

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35833.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline and AI policy for information on the review process.

If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #35768 (wallet: Reject whitespace-only wallet names by vicjuma)
• #25722 (refactor: Use util::Result class for wallet loading by ryanofsky)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

Just posting mostly as a curiosity to see if you looked at whether the logging interface is vulnerable to format string attacks. I don't think it is, but would be interested in knowing if so. Though I'm not aware of anywhere in the p2p interface that can arbitrarily log blobs.

I wonder if non-trailing b"\n" can be wholesale rejected by the log framework, unless they are opt-in for the IIRC single log line that "needs" them?

If someone does a followup that addresses that for the whole log framework, please ping me. Here I meant to cover the two exceptional cases I found of user-provided data that escapes sanitization.

We also print arbitrary stuff from the tor control server:

+1 to just blocking newlines in all logging, the tor control server can log control sequences except for those disallowed by LogEscapeMessage

https://github.com/bitcoin/bitcoin/blob/17c5e33e9c5418fb0240f5d4210c87f54b88cdda/src/torcontrol.cpp#L672

and from the i2p daemon:

https://github.com/bitcoin/bitcoin/blob/17c5e33e9c5418fb0240f5d4210c87f54b88cdda/src/i2p.cpp#L273-L275

Both of these can be remote in some setups (but are trusted i.e. if they are compromised you are in big trouble)

On top of newlines, there are some (obscure i believe) terminal setups where 8-bit control sequences > 127 can be printed which allows e.g. clipboard injection.

Maybe LogEscapeMessage like this:

diff --git a/src/logging.cpp b/src/logging.cpp
index bbe5f043ed..39e9493749 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -338,7 +338,9 @@ namespace BCLog {
         std::string ret;
         for (char ch_in : str) {
             uint8_t ch = (uint8_t)ch_in;
-            if ((ch >= 32 || ch == '\n') && ch != '\x7f') {
+            if (ch == '\n') {
+                continue;
+            } else if (ch >= 32 && ch < 127) {
                 ret += ch_in;
             } else {
                 ret += strprintf("\\x%02x", ch);
@@ -427,7 +429,7 @@ std::string BCLog::Logger::Format(const util::log::Entry& entry) const
     result += GetLogPrefix(static_cast<LogFlags>(entry.category), entry.level);
     result += LogEscapeMessage(entry.message);

-    if (!result.ends_with('\n')) result += '\n';
+    result += '\n';
     return result;
 }

Maybe LogEscapeMessage like this:

Sure, but this will silently break logs, as explained above. Ref:

# git grep --extended-regexp '(WalletLogPrintf\("CommitTransaction:\\n|LogWarning\("\\n)'
src/util/exception.cpp:    LogWarning("\n\n************************\n%s", message);
src/wallet/wallet.cpp:    WalletLogPrintf("CommitTransaction:\n%s\n", util::RemoveSuffixView(tx->ToString(), "\n"));

They will need to be fixed, or opt-in with an option, as explained above.

My recommendation would be to go ahead with this pull first, then rework the log framework in a separate pull.

I agree with Marco that the logging framework should be reworked separately, I’d like to keep this PR focused on the identified newline injection paths and handle broader filtering in follow-ups. And we should probably be careful about escaping every byte above 0x7f, since that could also affect valid UTF-8, including translated log messages.

ACK ed4eb51e9fc6f62975e272e96b22e0a6b64d3205