Merge bitcoin/bitcoin#35833: log: prevent user input from injecting fake log lines

### src/logging.cpp
@@ -19,6 +19,7 @@
 
 using util::Join;
 using util::RemovePrefixView;
+using util::RemoveSuffixView;
 
 const char * const DEFAULT_DEBUGLOGFILE = "debug.log";
 constexpr auto MAX_USER_SETABLE_SEVERITY_LEVEL{BCLog::Level::Info};
@@ -331,15 +332,16 @@ namespace BCLog {
     /** Belts and suspenders: make sure outgoing log messages don't contain
      * potentially suspicious characters, such as terminal control codes.
      *
-     * This escapes control characters except newline ('\n') in C syntax.
-     * It escapes instead of removes them to still allow for troubleshooting
-     * issues where they accidentally end up in strings.
+     * This escapes control characters, including newline ('\n'), in C syntax,
+     * so untrusted data can't forge log lines. It escapes instead of removes
+     * them to still allow for troubleshooting issues where they accidentally
+     * end up in strings.
      */
     std::string LogEscapeMessage(std::string_view str) {
         std::string ret;
         for (char ch_in : str) {
             uint8_t ch = (uint8_t)ch_in;
-            if ((ch >= 32 || ch == '\n') && ch != '\x7f') {
+            if (ch >= 32 && ch != '\x7f') {
                 ret += ch_in;
             } else {
                 ret += strprintf("\\x%02x", ch);
@@ -426,9 +428,9 @@ 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';
+    // Strip the conventional trailing '\n' that many callers still pass, so only embedded newlines are escaped
+    result += LogEscapeMessage(RemoveSuffixView(entry.message, "\n"));
+    result += '\n';
     return result;
 }
 

### src/test/util_tests.cpp
@@ -1398,8 +1398,8 @@ BOOST_AUTO_TEST_CASE(test_LogEscapeMessage)
 {
     // ASCII and UTF-8 must pass through unaltered.
     BOOST_CHECK_EQUAL(BCLog::LogEscapeMessage("Valid log message貓"), "Valid log message貓");
-    // Newlines must pass through unaltered.
-    BOOST_CHECK_EQUAL(BCLog::LogEscapeMessage("Message\n with newlines\n"), "Message\n with newlines\n");
+    // Newlines are escaped too, so a message can't forge log lines.
+    BOOST_CHECK_EQUAL(BCLog::LogEscapeMessage("Message\n with newlines\n"), R"(Message\x0a with newlines\x0a)");
     // Other control characters are escaped in C syntax.
     BOOST_CHECK_EQUAL(BCLog::LogEscapeMessage("\x01\x7f Corrupted log message\x0d"), R"(\x01\x7f Corrupted log message\x0d)");
     // Embedded NULL characters are escaped too.

### test/functional/rpc_whitelist.py
@@ -11,15 +11,17 @@
     str_to_b64str,
 )
 import http.client
+import json
 import urllib.parse
 
 
-def rpccall(node, user, method):
+def rpccall(node, user, method, *, batch=False):
     url = urllib.parse.urlparse(node.url)
     headers = {"Authorization": "Basic " + str_to_b64str('{}:{}'.format(user[0], user[3]))}
+    request = {"method": method}
     conn = http.client.HTTPConnection(url.hostname, url.port)
     conn.connect()
-    conn.request('POST', '/', '{"method": "' + method + '"}', headers)
+    conn.request('POST', '/', json.dumps([request] if batch else request), headers)
     resp = conn.getresponse()
     conn.close()
     return resp
@@ -92,6 +94,7 @@ def run_test(self):
 
         self.test_users_permissions()
         self.test_rpcwhitelistdefault_permissions(0, 200)
+        self.test_rejected_method_logging()
 
         # Replace file configurations
         self.nodes[0].replace_in_config([("rpcwhitelistdefault=0", "rpcwhitelistdefault=1")])
@@ -110,6 +113,16 @@ def run_test(self):
         self.test_users_permissions()
         self.test_rpcwhitelistdefault_permissions(1, 403)
 
+    def test_rejected_method_logging(self):
+        """Test logging a method rejected by the RPC whitelist."""
+        self.log.info(f"[{self.users[0][0]}]: Testing rejected method logging")
+        forged_log_line = "ERROR: ConnectTip: ConnectBlock 0000000000000000deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef failed, bad-txns-inputs-missingorspent"
+        # Stripping special characters could make the log appear to reject a whitelisted method
+        for rejected_method in [f"getblock\n{forged_log_line}", 'getblock!"#$%&\'*+<=>[\\]^`{|}~']:
+            for batch in [False, True]:
+                with self.nodes[0].assert_debug_log([f"not allowed to call method {rejected_method}".replace("\n", "\\x0a")]):
+                    assert_equal(403, rpccall(self.nodes[0], self.users[0], rejected_method, batch=batch).status)
+
     def test_users_permissions(self):
         """
         * Permissions:

### test/functional/wallet_startup.py
@@ -7,6 +7,7 @@
 Verify that a bitcoind node can maintain list of wallets loading on startup
 """
 import os
+import platform
 import shutil
 import stat
 import uuid
@@ -93,6 +94,26 @@ def test_disabled_settings(self, node):
         self.start_node(0)
         assert_equal(set(node.listwallets()), {'w2', 'w3'})
 
+    def test_startup_warning_log_injection(self, node):
+        self.log.info("Test that a wallet name cannot forge log lines through startup warnings")
+        forged_log_line = "ERROR: ConnectTip: ConnectBlock 0000000000000000deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef failed, bad-txns-inputs-missingorspent"
+        wallet_name = f"w5\n{forged_log_line}"
+        if platform.system() == 'Windows':
+            # Windows disallows newlines in filenames
+            assert_raises_rpc_error(None, None, node.createwallet, wallet_name, load_on_startup=True)
+            return
+
+        with node.assert_debug_log([f"[{wallet_name}]".replace("\n", "\\x0a")]):
+            assert_equal(node.createwallet(wallet_name=wallet_name, load_on_startup=True)["name"], wallet_name)
+        self.stop_node(0)
+
+        wallet_path = node.wallets_path / wallet_name
+        self.cleanup_folder(wallet_path)
+        warning = f"Skipping -wallet path that doesn't exist. Failed to load database path '{wallet_path}'. Path does not exist."
+        with node.assert_debug_log([warning.replace("\n", "\\x0a")], unexpected_msgs=[f"[warning] {forged_log_line}"]):
+            self.start_node(0)
+        self.stop_node(0, expected_stderr=f"Warning: {warning}")
+
     def run_test(self):
         self.log.info('Should start without any wallets')
         assert_equal(self.nodes[0].listwallets(), [])
@@ -124,6 +145,7 @@ def run_test(self):
 
         self.test_load_unwritable_wallet(self.nodes[0])
         self.test_disabled_settings(self.nodes[0])
+        self.test_startup_warning_log_injection(self.nodes[0])
 
 if __name__ == '__main__':
     WalletStartupTest(__file__).main()