util: keep wallet names literal in notification commands #36048

pull l0rinc wants to merge 4 commits into bitcoin:master from l0rinc:l0rinc/walletnotify-literal-replacement changing 4 files +40 −13

l0rinc commented at 8:30 PM on August 20, 2026: contributor

Problem: On non-Windows builds, operators can configure -walletnotify to run a command for wallet transactions, with %w replaced by the shell-escaped wallet name. An authenticated RPC caller allowed to create wallets can supply a name containing $', request an address, and send a transaction to it. While replacing %w, ReplaceAll() passes the escaped wallet name to std::regex_replace() as replacement text, where $' copies the command suffix into the escaped name and allows its shell metacharacters to alter the command. runCommand() passes the result to system(), so a suitable command template could execute additional shell commands as the node process account. It is not reachable over P2P or by an unauthenticated network peer. #25803 introduced this behavior in v24 when it replaced Boost's literal substitution with std::regex_replace().

Fix: Restore the literal, non-recursive contract ReplaceAll() had before #25803, matching every current caller's literal search and replacement text, while the wallet notification test covers a wallet name containing $'.

Related: #35833 restricts control characters in new wallet names, while this change fixes replacement metacharacters in ReplaceAll().

This was found and disclosed responsibly by the Red Team 🟥.

test: simplify `ReplaceAll` coverage

Let each case provide its input so strings outside the original fixture can use the same table without separate temporary variables.

6d2aeb096d

test: characterize walletnotify shell injection

`-walletnotify` shell-escapes wallet names before substituting `%w` into the configured command.
`ReplaceAll()` uses `%w` as the regex pattern and the escaped wallet name as replacement text, where `$'` copies the command suffix into the escaped name and allows its shell metacharacters to alter the command.

Record the command execution, missing notification file, regex pattern matching, replacement expansion, and non-recursive replacement.

4512a1eab9

util: make `ReplaceAll` literal

`ReplaceAll()` substitutes fixed tokens in notification commands and other strings.
PR #25803 replaced the Boost helper with `std::regex_replace()`, treating searches as regular expressions and substitutes as replacement-format syntax.

Restore literal, non-recursive replacement so callers match fixed tokens and preserve replacement bytes exactly, while avoiding a new string when the search text is absent.

Co-authored-by: Rob Hamilton <6456095+Rob1Ham@users.noreply.github.com>

526053e038

refactor: use string views in `ReplaceAll`

PR #25803 changed these parameters to `const std::string&` for `std::regex_replace()`.
The literal implementation no longer needs owned strings, so restore the original `std::string_view` interface.

c88468d2ae

DrahtBot added the label Utils/log/libs on Aug 20, 2026
DrahtBot commented at 8:31 PM on August 20, 2026: contributor



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



Code Coverage & Benchmarks

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



Reviews

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

Type Reviewers

Concept ACK achow101

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.
achow101 commented at 9:08 PM on August 20, 2026: member

Concept ACK

Type	Reviewers
Concept ACK	achow101

Contributors

Labels

Linked (view graph)

#25803 refactor: Drop `boost/algorithm/string/replace.hpp` dependency #35833 rpc,wallet: prevent user input injecting fake log lines

util: keep wallet names literal in notification commands #36048

Code Coverage & Benchmarks

Reviews