CODEWITHSUNDEEP
c#asp.netfortify
The Birdman
The Birdman

Reputation: 61

Open Filestream For Writing - fortify Path Manipulation

Looking to resolve a fortify finding (Path Manipulation) for resolving opening a file:

public FileStream OpenFile(string directory, string filename)
{
    FileStream fs = null;
    string pathname = string.Empty;
    pathname = Path.Combine(directory, filename);
    fs = new FileStream(pathname , FileMode.OpenOrCreate);
    return (fs);
}

This code runs in a .NET application, but DOES NOT write to a virtual directory.

The Fortify help / suggestion indicates white listing the valid directories, but that is tantamount to hard coding the directories in the application. It may be secure, but it is not a good programming practice.

Thanks in advance

Upvotes: 4

Views: 6172

Answers (2)

thosoc
thosoc

Reputation: 1

You will need to add code to check "directory" and "pathname" to ensure of their existences on the system before calling FileStream. For .NET, you can use the stat() function for the check.

Upvotes: 0

Douglas Held
Douglas Held

Reputation: 1461

@James Nix has provided the reason Fortify found a vulnerability (in a comment):

You are getting this finding because this method accepts a "user provided" path and file name. If an attacker were to send this method the parameters directory=C:\Windows and filename=notepad.exe they could overwrite notepad.exe with something malicious if your application had write permissions to that file. – James Nix Jan 6 at 17:17

If you are interested in fixing the vulnerabilities, then you will need to:

  1. If possible, change method signature from public to private.
  2. Provide a fixed prefix to the provided file path (like "D:\Temp\" or Application("files_root") which you could add to your application configuration.
  3. Do not allow "/" or "\" or ".." or ":" in the filename argument. Or just restrict to something like 8.3 if appropriate.
  4. Do not allow ".." or ":" in the path argument. Or just restrict to the acceptable character range (a-z for instance).
  5. Do not return the open FileStream object. You lose control over whether it is ever closed (Denial of Service vulnerability). Instead, get the data you need and close the FileStream before returning from the method.

If you want more targeted remediation advice, you need to describe what your application needs to do with this method.

Upvotes: 1

Related Questions