Path traversal warning when using Path.Combine

Question

I am currently using NewtonJSON to load some UI data from a JSON file. However, there is an warning says I have path traversal.

Here is the situation:

Any idea to remove this security vulnerability?

Answer 1

The path traversal exploit is possible when a path provided by a user or other untrusted source is combined, without checking, with a parent path. The problem is that there are "path traversal" components of paths that enable navigation out of the parent folder.

For example, if you were to combine the following two paths, an absolute path and a relative one:

Base/absolute path: C:\WebData
Relative/user path: ..\windows\system32\

Then this would yield:

C:\windows\system32\

As you can imagine, letting someone read or write this directory, which is not the intended WebData directory, could be a huge problem, as it could lead to someone learning information about your system or placing exploits that compromise the system, giving control of it to malicious actors.

You can read more about this exploit.

To deal with this vulnerability properly, you need to ensure that the relative path combined with the parent path is safe. Here are some ways to ensure this:

• The relative path comes from a known trusted source, such as a vetted, internal list.
• The relative path has previously been checked and was stored/maintained in a way that ensures it can be distinguished from untrusted paths, and could not be modified by a user or untrusted agent in the meantime. For example, keeping the path in a string is a bad idea. Instead, you would do something like create a TrustedPath class that code could only gain an instance of by, in fact, running code that checked that the path is safe.
• The resulting path is checked after combination to ensure it is in the correct location.

You could do the last item like so (all of the below):

1. (As good practice to avoid unnecessary exceptions), use Path. GetInvalidFileNameChars() to check the (untrusted) relative path for invalid characters.
2. Perform Path.Combine() as you have done.
3. Ensure the resulting path is still within the original, parent path. This can be done by simply ensuring the resulting path starts with the parent path, but there may be issues with that. So consider an answer like this or other code that ensures the resulting path is truly a descendant of the desired folder.

Once you have done all this, if the "path traversal" warning is still showing, you can use the menu options in your code quality/security checking tool to annotate this instance of path traversal as safe. You may also want to put a comment with notes explaining why you marked it as safe, which could conceivably include a link to this SO question or one of its answers.

Note 1

Be careful about reusing a relative path that you have proven combines okay with a specific absolute path. Consider the following:

Base/absolute path: C:\WebData\FormElements\SuperForm\windows\
Relative/user path: ..\windows\

These two paths would combine safely, however this has not proven that the relative path is always safe to use with any absolute path.

Note 2

Be wary how you go about this. Assuming that relative path traversal always starts with ..\ is a mistake. The following is a valid relative path: folder\..\..\wheeeWeGotOut.

Note 3

Another answer proposes that safety can be guaranteed by simply removing invalid characters, and disallowing paths that contain .. or :. This is problematic for multiple reasons:

• Files in many non-Windows file systems such as HFS or linux can legitimately have these characters. For example, a:filename and another..filename are perfectly fine (I just tested them). Restricting those characters limits what users can do.

• Trying to improve the filtering to allow legitimate use-cases through is not a good idea. How do you know that you've written this code correctly and didn't miss an edge-case?

• But most of all: what if there is an accidental symlink inside of the user's allowed path that points to a file elsewhere in the filesystem? What if the user is able to write a file that can function as a symlink, or has another exploit to cause such a file to be written or copied? The path exploit may not stand alone. It is often a combination of small exploits that leads to large exploits (each next one intersecting with or escalating the previous one). The only safe technique to ensure a file or directory is in the proper location is to, after all other filtering and passes and combining have been done, to check that the result is still within the expected location or is a direct descendant (paying attention to symlinks).

• Regarding hard links—that is another matter. Good luck with that one. Don't make hard links. It's very hard to tell that a hard-link even exists, because it's a low-level modification. Read up on it if you're interested.

Answer 2

A secure way of using Path.Combine can be the next:

    public string PathCombine(string path1, string path2)
    {
        if (path2.Contains("..")) return null;
        if (path2.Contains(":")) return null;

        string result = Path.Combine(path1, path2);

        return (result.Equals(path2) ? null : result);
    }

Note that this is only an example, not allowed strings check can be improved. As additional information you can take a look here. If you want to get more information about how this problem can be exploited I recommend you to read the OWASP documentation.

The fact of check if the result is like the second parameter is due to the behavior of the CombineInternal function that you can check here:

    private static string CombineInternal(string first, string second)
    {
        if (string.IsNullOrEmpty(first))
            return second;

        if (string.IsNullOrEmpty(second))
            return first;

        if (IsPathRooted(second.AsSpan()))
            return second;

        return JoinInternal(first.AsSpan(), second.AsSpan());
    }

As you can see, if the second variable IsPathRooted, the result is going to be it, and this is a common way to explode this without the need of use '..' characters, think on this example:

Your web is on c:\wwwroot\web1\public\index.html if you pass as second parameter c:\wwwroot\web1\private\secret.conf you are going to be able to access this file.