Does my code prevent directory traversal or is it overkill?

Question

I want to make sure this is enough to prevent directory traversal and also any suggestions or tips would be appreciated. The directory "/wwwroot/Posts/" is the only directory which is allowed.

    [HttpGet("/[controller]/[action]/{name}")]
    public IActionResult Post(string name)
    {
        if(string.IsNullOrEmpty(name))
        {
            return View("Post", new BlogPostViewModel(true)); //error page
        }

        char[] InvalidFilenameChars = Path.GetInvalidFileNameChars();

        if (name.IndexOfAny(InvalidFilenameChars) >= 0)
        {
            return View("Post", new BlogPostViewModel(true));
        }

        DirectoryInfo dir = new DirectoryInfo(Path.Combine(Directory.GetCurrentDirectory(), "wwwroot/Posts"));

        var userpath = Path.GetFullPath(Path.Combine(Directory.GetCurrentDirectory(), "wwwroot/Posts", name));

        if (Path.GetDirectoryName(userpath) != dir.FullName)
        {
            return View("Post", new BlogPostViewModel(true));
        }

        var temp = Path.Combine(dir.FullName, name + ".html");
        if (!System.IO.File.Exists(temp))
        {
            return View("Post", new BlogPostViewModel(true));
        }
        BlogPostViewModel model = new BlogPostViewModel(Directory.GetCurrentDirectory(), name);
        return View("Post", model);
    }

Answer 1

Probably, but I wouldn't consider it bulletproof. Let's break this down:

First you are black-listing known invalid characters:

    char[] InvalidFilenameChars = Path.GetInvalidFileNameChars();

    if (name.IndexOfAny(InvalidFilenameChars) >= 0)
    {
        return View("Post", new BlogPostViewModel(true));
    }

This is a good first step, but blacklisting input is rarely enough. It will prevent certain control characters, but the documentation does not explicitly state that directory separators ( e.g. / and \) are included. The documentation states:

The array returned from this method is not guaranteed to contain the complete set of characters that are invalid in file and directory names. The full set of invalid characters can vary by file system.

Next, you attempt to make sure that after path.combine you have the expected parent folder for your file:

    DirectoryInfo dir = new DirectoryInfo(Path.Combine(Directory.GetCurrentDirectory(), "wwwroot/Posts"));

    var userpath = Path.GetFullPath(Path.Combine(Directory.GetCurrentDirectory(), "wwwroot/Posts", name));

    if (Path.GetDirectoryName(userpath) != dir.FullName)
    {
        return View("Post", new BlogPostViewModel(true));
    }

In theory, if the attacker passed in ../foo (and perhaps that gets past the blacklisting attempt above if / isn't in the list of invalid characters), then Path.Combine should combine the paths and return /somerootpath/wwwroot/foo. GetParentFolder would return /somerootpath/wwwroot which would be a non-match and it would get rejected. However, suppose Path.Combine concatenates and returns /somerootpath/wwwroot/Posts/../foo. In this case GetParentFolder will return /somerootpath/wwwRoot/Posts which is a match and it proceeds. Seems unlikely, but there may be control characters which get past GetInvalidFileNameChars() based on the documentation stating that it is not exhaustive which trick Path.Combine into something along these lines.

Your approach will probably work. However, if it is at all possible, I would strongly recommend you whitelist the expected input rather than attempt to blacklist all possible invalid inputs. For example, if you can be certain that all valid filenames will be made up of letters, numbers, and underscores, build a regular expression that asserts that and check before continuing. Testing for ^[A-Za-z0-0_]+$ would assert that and be 100% bulletproof.