Merge bitcoin#25043: Reject invalid rpcauth formats

fa12706 Reject invalid rpcauth formats (MacroFake)

Pull request description:

  This was added in commit 438ee59, but I couldn't determine if it was intentional.

  One reason to accept `foo:bar:baz` over `foo:bar$baz` is that `$` may be eaten by the shell. Though, I don't think many users pass `rpcauth` via the shell. Also it should be easy to avoid by passing `'-rpcauth=foo:bar$baz'` or `"-rpcauth=foo:bar\$baz"`.

  Can be tested with the added test.

ACKs for top commit:
  pk-b2:
    ACK fa12706

Tree-SHA512: 9998cbb295c79f7b0342bf86e1d3e5b5ab90851c627662ad6495b699a65a9035998173cf1debfd94325387faba184de683407b609fe86acdd8f6749157644441

Author: fanquake

src/httprpc.cpp

@@ -4,15 +4,13 @@
 
 #include <httprpc.h>
 
-#include <chainparams.h>
 #include <crypto/hmac_sha256.h>
 #include <httpserver.h>
 #include <rpc/protocol.h>
 #include <rpc/server.h>
 #include <util/strencodings.h>
 #include <util/string.h>
 #include <util/system.h>
-#include <util/translation.h>
 #include <walletinitinterface.h>
 
 #include <algorithm>
@@ -21,6 +19,7 @@
 #include <memory>
 #include <set>
 #include <string>
+#include <vector>
 
 /** WWW-Authenticate to present with 401 Unauthorized response */
 static const char* WWW_AUTH_HEADER_DATA = "Basic realm=\"jsonrpc\"";
@@ -288,12 +287,14 @@ static bool InitRPCAuthentication()
         LogPrintf("Config options rpcuser and rpcpassword will soon be deprecated. Locally-run instances may remove rpcuser to use cookie-based auth, or may be replaced with rpcauth. Please see share/rpcauth for rpcauth auth generation.\n");
         strRPCUserColonPass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-rpcpassword", "");
     }
-    if (gArgs.GetArg("-rpcauth","") != "")
-    {
+    if (gArgs.GetArg("-rpcauth", "") != "") {
         LogPrintf("Using rpcauth authentication.\n");
         for (const std::string& rpcauth : gArgs.GetArgs("-rpcauth")) {
-            std::vector<std::string> fields = SplitString(rpcauth, ":$");
-            if (fields.size() == 3) {
+            std::vector<std::string> fields{SplitString(rpcauth, ':')};
+            const std::vector<std::string> salt_hmac{SplitString(fields.back(), '$')};
+            if (fields.size() == 2 && salt_hmac.size() == 2) {
+                fields.pop_back();
+                fields.insert(fields.end(), salt_hmac.begin(), salt_hmac.end());
                 g_rpcauth.push_back(fields);
             } else {
                 LogPrintf("Invalid -rpcauth argument.\n");

test/functional/rpc_users.py

@@ -106,6 +106,9 @@ def run_test(self):
         self.stop_node(0)
         self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=foo'])
         self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=foo:bar'])
+        self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=foo:bar:baz'])
+        self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=foo$bar:baz'])
+        self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=foo$bar$baz'])
 
         self.log.info('Check that failure to write cookie file will abort the node gracefully')
         cookie_file = os.path.join(get_datadir_path(self.options.tmpdir, 0), self.chain, '.cookie.tmp')