csharpsquid:S5445 Usage of GetTempFileName

SonarQube Server / Community Build
,
1

Hello SonarQube community,

I have a problem understanding the rule csharpsquid:S5445, “Insecure temporary file creation methods should not be used“

It advises against using GetTempFileName. The reason given is: “Temporary files are considered insecurely created when the file existence check is performed separately from the actual file creation.”

However, as I understand it, this should not be a problem with GetTempFileName, as a clean CreateNew is used internally:

File.OpenHandle(path, FileMode.CreateNew, FileAccess.Write).Dispose();

So, no time between check and creation of that file.
What exactly is the problem here that SonarQube wants to point out?

SonarQube Server: Enterprise Edition v2025.4.1 (111832)

3

Hi @stephans, and welcome to the community!

You’re right to question this, as your understanding of how GetTempFileName works internally is correct. It does atomically create a zero-byte file using FileMode.CreateNew in Path.Windows.cs#L252.

However, what the SonarQube rule wants to point out is not the creation of that initial file; it’s in the gap between that creation and when your application re-opens the file to use it. I think the problem here is our explanation of the security issue within the text. I am going to take a look at the implementation of this rule and on changing the text.

Here is an explanation of what the rule wanted to point out:

The (insecure) workflow that Path.GetTempFileName() encourages is a non-atomic, two-step operation:

  1. Step 1 (Atomic): Path.GetTempFileName() is called. It securely creates an empty, zero-byte file (e.g., C:\Temp\tmpA1B2.tmp). You get the path back as a string.

  2. Step 2 (The Gap): Your code then takes that string and opens the file again to write to it, for example, with new StreamWriter(tempPath).

The security risk the race condition exists in the tiny window of time between Step 1 and Step 2. This is called a TOCTOU:

TOCTOU (time-of-check to time-of-use) is a type of race condition where the state of a system or resource is checked, and then used, but the state changes between those two actions, allowing for an exploit. This vulnerability allows attackers to change a resource, such as a file or configuration, between the time a program (1) creates/validates it and (2) when it actually uses it.

The compliant solution avoids this two-step process entirely. It performs the file creation and opening in a single, atomic operation.

// This generates a NAME ONLY. No file is created yet.
var randomPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());

// This is the key:
using (var fileStream = new FileStream(randomPath, 
                                       FileMode.CreateNew, // Fails if file exists
                                       FileAccess.Write,
                                       FileShare.None,     // Lock the file
                                       4096,
                                       FileOptions.DeleteOnClose))
{
    // You get a ready-to-use stream from the one and only file operation.
    // There is NO gap for an attacker to exploit.
    using (var writer = new StreamWriter(fileStream))
    {
        writer.WriteLine("content");
    }
}

Thanks a lot for your feedback!

Loris

4

Hello Loris,

thank you for your reply. The temporary file is created in the user’s context, which means that, in theory, the user always has full access to this file.

Furthermore, this file is usually “overwritten” using File.WriteAllText or similar methods. I find it difficult to identify an attack vector here. Is overwritting the file a FalsePositive?

The functionality of GetTempFileName, which guarantees a free temp file name, is very appealing and can only be indirectly replicated with GetRandomFileName (larger namespace).

Regards
Stephan

5

So, first in terms of permissions, the “attacker” in this scenario is often a malicious process running with the same permissions, or a superset of permissions.

The threat isn’t necessarily another user on a multi-tenant system. Instead, it could be:

  • A compromised dependency in another application.

  • A malicious script running in the background.

  • A host intrusion

And then about the attack, I should have written this in my previous response. The easiest one is just modifying the content and then exploiting internal application logic depending on what you do with the new file.

However, the most critical attack is about redirecting your file operation to a different, sensitive file, with a symbolic link attack.

In that case, File.WriteAllText is particularly dangerous because it completely replaces a file’s content.

Here is the attack scenario with File.WriteAllText:

  1. Your App Calls Path.GetTempFileName(). A zero-byte file, C:\Temp\tmpABC.tmp, is created.

  2. A malicious process (running with the proper permissions) is monitoring the temp folder. It instantly deletes tmpABC.tmp.

  3. The attacker immediately creates a symbolic link also named tmpABC.tmp. This link doesn’t point to a normal file; it points to a critical file the user owns, like C:\Users\YourUser\.ssh\authorized_keys or a configuration file for another application.

  4. Your App Calls File.WriteAllText("C:\\Temp\\tmpABC.tmp", "some-new-content").

  5. Your application believes it’s overwriting its temporary file. However, the operating system follows the symbolic link, and your app actually overwrites the authorized_keys file. The attacker may have just granted themselves persistent SSH access to the user’s account.

I hope this clarifies, we should definitely mention that in the rule.

Edit: In your case of course it’s a full deletion and new contents might not be controlled by an attacker, so this vulnerability could lead to other risks. Deleting important permission configuration files, creating a DOS, etc.