CodeMonkey
CodeMonkey

Reputation: 19

Security flaw relating to special characters in Windows file paths

I may have found a security flaw in my Windows application. It takes in a string from the user and applies the following logic:

string fileName = userInput + ".notSecret";
return new FileStream(fileName, FileMode.Open, FileAccess.Read);

Is it possible for the user to input any characters that would allow them to enter a new file extension by terminating the file name early?

For example:

"secret_file.Secret\0"

Upvotes: 1

Views: 497

Answers (1)

drf
drf

Reputation: 8709

The scenario you indicate is most likely not a security flaw. First, .NET strings are not null-truncated and may contain null characters. Moreover, if you try to create a file in the StreamWriter constructor with a control character, including \0, you will find it throws an exception:

Unhandled Exception: System.ArgumentException: Illegal characters in path.
   at System.IO.Path.CheckInvalidPathChars(String path, Boolean checkAdditional)
   at System.IO.Path.GetFileName(String path)
   at System.IO.StreamWriter.CreateFile(String path, Boolean append, Boolean checkHost)
   at System.IO.StreamWriter..ctor(String path, Boolean append, Encoding encoding, Int32 bufferSize, Boolean checkHost)
   at System.IO.StreamWriter..ctor(String path)

Note that there would be a security hole if the .NET IO framework supported alternate data streams; a user could write to "userInput.secret:.notSecret", coercing a write to an alternate data stream of "userInput.secret". Since System.IO classes do not support alternate data streams, a NotSupportedException is thrown and the rogue file is not written.

There are potential file system implications inherent in using the file extension as a security control. For instance, if the user has a file with the extension .notSecret mapped as a hard link to a file with the extension .secret, writing to one would also write to the other (since in a hard link, a single file can have multiple paths with different extensions).

Assuming that userInput is raw code provided by the user, the code fails to sanitize the user input. When handling input from untrusted sources, code should assume all user input is evil until proven otherwise. If the user's input is expected to be a certain value (e.g., no special characters), code handling the input should ensure it matches, and sanitize it (or throw an exception) if it does not. Allowing only inputs known to be good is much safer than only disallowing inputs known to be bad.

Upvotes: 1

Related Questions