Repository: bitcoin/bitcoin
File: src/rpc/request.cpp (C++)

Code before:
    error.pushKV("message", message);
    return error;
}

/** Username used when cookie authentication is in use (arbitrary, only for
 * recognizability in debugging/logging purposes)
 */
static const std::string COOKIEAUTH_USER = "__cookie__";
/** Default name for auth cookie file */
static const char* const COOKIEAUTH_FILE = ".cookie";

/** Get name of RPC authentication cookie file */
static fs::path GetAuthCookieFile(bool temp=false)
{
    fs::path arg = gArgs.GetPathArg("-rpccookiefile", COOKIEAUTH_FILE);
    if (temp) {
        arg += ".tmp";
    }
    return AbsPathForConfigVal(gArgs, arg);
}

static bool g_generated_cookie = false;

bool GenerateAuthCookie(std::string* cookie_out, std::optional<fs::perms> cookie_perms)
{
    const size_t COOKIE_SIZE = 32;
    unsigned char rand_pwd[COOKIE_SIZE];
    GetRandBytes(rand_pwd);
    std::string cookie = COOKIEAUTH_USER + ":" + HexStr(rand_pwd);

    /** the umask determines what permissions are used to create this file -
     * these are set to 0077 in common/system.cpp.
     */
    std::ofstream file;
    fs::path filepath_tmp = GetAuthCookieFile(true);
    file.open(filepath_tmp);
    if (!file.is_open()) {
        LogInfo("Unable to open cookie authentication file %s for writing\n", fs::PathToString(filepath_tmp));
        return false;
    }
    file << cookie;
    file.close();

    fs::path filepath = GetAuthCookieFile(false);
    if (!RenameOver(filepath_tmp, filepath)) {
        LogInfo("Unable to rename cookie authentication file %s to %s\n", fs::PathToString(filepath_tmp), fs::PathToString(filepath));
        return false;
    }
    if (cookie_perms) {
        std::error_code code;
        fs::permissions(filepath, cookie_perms.value(), fs::perm_options::replace, code);

Review comment (security):
In commit "args: Support -norpccookiefile" (cf3e4edc803bf950f280b24c5283a2838c86faf6)

I think having two separate accesses to the -rpccookiefile in different functions is fragile since `GetAuthCookieFile` will not do the right thing if called in a different context. It is also not taking advantage of GetPathArg's built in handling of negation. I think a change like:

```diff
--- a/src/rpc/request.cpp
+++ b/src/rpc/request.cpp
@@ -86,6 +86,7 @@ static const char* const COOKIEAUTH_FILE = ".cookie";
 static fs::path GetAuthCookieFile(bool temp=false)
 {
     fs::path arg = gArgs.GetPathArg("-rpccookiefile", COOKIEAUTH_FILE);
+    if (arg.empty()) return {}; // -norpccookiefile was specified
     if (temp) {
         arg += ".tmp";
     }
@@ -96,11 +97,6 @@ static bool g_generated_cookie = false;
 
 bool GenerateAuthCookie(std::string* cookie_out, std::optional<fs::perms> cookie_perms)
 {
-    if (gArgs.IsArgNegated("-rpccookiefile")) {
-        LogInfo("RPC authentication cookie file generation is disabled.");
-        return true;
-    }
-
     const size_t COOKIE_SIZE = 32;
     unsigned char rand_pwd[COOKIE_SIZE];
     GetRandBytes(rand_pwd);
@@ -111,6 +107,11 @@ bool GenerateAuthCookie(std::string* cookie_out, std::optional<fs::perms> cookie
      */
     std::ofstream file;
     fs::path filepath_tmp = GetAuthCookieFile(true);
+    if (filepath_tmp.empty()) {
+        LogInfo("RPC authentication cookie file generation is disabled.");
+        return true;
+    }
+
     file.open(filepath_tmp);
     if (!file.is_open()) {
         LogInfo("Unable to open cookie authentication file %s for writing\n", fs::PathToString(filepath_tmp));

```

would make this safer and clearer.

Code after:
    error.pushKV("message", message);
    return error;
}

/** Username used when cookie authentication is in use (arbitrary, only for
 * recognizability in debugging/logging purposes)
 */
static const std::string COOKIEAUTH_USER = "__cookie__";
/** Default name for auth cookie file */
static const char* const COOKIEAUTH_FILE = ".cookie";

/** Get name of RPC authentication cookie file */
static fs::path GetAuthCookieFile(bool temp=false)
{
    fs::path arg = gArgs.GetPathArg("-rpccookiefile", COOKIEAUTH_FILE);
    if (arg.empty()) {
        return {}; // -norpccookiefile was specified
    }
    if (temp) {
        arg += ".tmp";
    }
    return AbsPathForConfigVal(gArgs, arg);
}

static bool g_generated_cookie = false;

bool GenerateAuthCookie(std::string* cookie_out, std::optional<fs::perms> cookie_perms)
{
    const size_t COOKIE_SIZE = 32;
    unsigned char rand_pwd[COOKIE_SIZE];
    GetRandBytes(rand_pwd);
    std::string cookie = COOKIEAUTH_USER + ":" + HexStr(rand_pwd);

    /** the umask determines what permissions are used to create this file -
     * these are set to 0077 in common/system.cpp.
     */
    std::ofstream file;
    fs::path filepath_tmp = GetAuthCookieFile(true);
    if (filepath_tmp.empty()) {
        return true; // -norpccookiefile
    }
    file.open(filepath_tmp);
    if (!file.is_open()) {
        LogWarning("Unable to open cookie authentication file %s for writing", fs::PathToString(filepath_tmp));
        return false;
    }
    file << cookie;
    file.close();

    fs::path filepath = GetAuthCookieFile(false);
    if (!RenameOver(filepath_tmp, filepath)) {