2

I'm analyzing my code (C#, desktop application) with CAT.NET Code Analysis and getting "Sanitize the file path prior to passing it to file system routines" message when dealing with file names. What I don't understand is that to ensure the file name is valid, I use:

void SomeMethod(String filename)
{
    filename = System.IO.Path.GetFullPath(filename);
    // ... Do stuff
}

Isn't it a "magic solution" to solve problems with invalid file names ? I've read something similar here (first answer), but in my case I'm dealing only with local files, well, something very basic, so...

So why I'm getting this message and how to do to avoid getting it?

Community
  • 1
  • 1
Arseni Mourzenko
  • 45,791
  • 28
  • 101
  • 185

2 Answers2

1

I know this is an old question, but I've just come across something that may be helpful specifically related to the CAT.Net error message.

In a blog post about the CAT.Net Data Flow Rules, they have this to say about the FileCanonicalizationRule:

Description

User input used in the file handling routines can potentially lead to File Canonicalization vulnerability. Code is particularly susceptible to canonicalization issues if it makes any decisions based on the name of a resource that is passed to the program as input. Files, paths, and URLs are resource types that are vulnerable to canonicalization because in each case there are many different ways to represent the same name.

Resolution

Sanitize the file path prior to passing it to file handling routines. Use Path.GetInvalidFileNameChars or Path.GetInvalidPathChars to get the invalid characters and remove them from the input. More information can be found at http://msdn.microsoft.com/en-us/library/system.io.path.getinvalidfilenamechars.aspx.

So, they suggest that you use Path.GetInvalidFileNameChars and Path.GetInvalidPathChars to validate your paths.

Note that their suggestion is to remove the invalid characters. While this will indeed make the path valid, it may cause unexpected behaviour for the user. As the comments on this question/answer suggest it's probably better to quit early and tell the user that their path is invalid, rather than doing something unexpected with their input (like removing bad characters and using the modified version).

Community
  • 1
  • 1
Richard Marskell - Drackir
  • 12,382
  • 3
  • 30
  • 53
0

If the filename comes from a user, it could be something like "../../../../etc/passwd" - the error message is telling you that you need to sanitize it so that it can't get to directories it's not supposed to.

Annie
  • 6,575
  • 20
  • 27
  • That's exactly why I use Path.GetFullPath, which will transform a path like "C:\SomeStuff\ChildDirectory\..\..\FileHere.txt" to "C:\FileHere.txt". – Arseni Mourzenko Dec 31 '09 at 07:53
  • And that is a huge security risk if the user entered the path--they could possibly access/overwrite files in directories they shouldn't be allowed to touch. In your example, you're assuming the file will be in "C:\SomeStuff\ChildDirectory", but really they're accessing a file in "C:\". – Annie Dec 31 '09 at 08:04
  • I really don't understand. Path.GetFullPath("C:\SomeStuff\ChildDirectory\..\..\FileHere.txt") gives "C:\FileHere.txt". So by calling GetFullPath() at the beginning of each method (when need) and using its result, I'm safe. Or not? – Arseni Mourzenko Dec 31 '09 at 11:20
  • If you just call GetFullPath with a user-supplied filename, you allow the user to access *any* path on the machine, not just the paths they are supposed to access. For example, lets say your app allows users to upload images, and stores them to c:\MyApp\Images, with the name the user specifies. They specify the name "..\..\Windows\explorer.exe" and overwrite C:\Windows\explorer.exe with a trojan. So you need to sanitize out things like "..\" instead of just accepting them into GetFullPath(). – Annie Jan 01 '10 at 22:38