Are You Serving Files Insecurely in ASP.NET

Posted by Filip Ekberg on 12 Jul 2013

I recently came across something quite interesting when it comes to serving files from your application. In theory what will be discussed here applies to all programming languages that you use for web programming and not only ASP.NET. However the examples used below will be targeted at ASP.NET developers.

The scenario

Let's say that you have system where customers can logon to download their invoices. We're using ASP.NET MVC and the action that retrieves our list of invoices is quite simple, it looks like this:

public ActionResult MyInvoices()
{ 
    // Get the file names from the persistent store
    var documents = new[] { "Filip_20130712.pdf", "Filip_20130713.pdf", "Filip_20130714.pdf" };

    return View(documents);
}

In reality we would've fetched the document names from the database, or maybe even just fetching the invoice dates and concatenating the file names ourselves. The view that shows the user the list of invoices and lets the user download them is just as simple as the code fetching the documents:

<h2>MyInvoices</h2>
<ul>
    @foreach (var document in Model)
    { 
        <li><a href="/Home/DownloadDocument?document=@document">Download @document</a></li>
    }
</ul>

This gives us the following beautiful website that shows a list of our invoices:

Now let's take a look at the implementation of the method that lets us download the invoice. Here's how I want it to work:

• Verify that the document requested exists on the server
• Feed the document to the client

Simple enough, right?

Simply enough we can use Server.MapPath to get the current directory of our application and then use Path.Combine to ask for the document in our Invoices folder.

public ActionResult DownloadDocument(string document)
{
    var documentPath = Server.MapPath(Path.Combine("Invoices", document));

    if (!System.IO.File.Exists(documentPath))
    {
        return null;
    }

    return File(documentPath, "application/pdf", document);
}

Now, if we run this application and navigate to /Home/MyInvoices we're going to see the list that we saw in the screenshot above. If we click the first document and request it and at the same time debug the application, we're going to see that it requests a file from \Home\Invoices\. So far so good, right?

Security?

You might have already figured out that there's a huge security risk with the above demo, let's try something crazy. We know that it combines the text that we pass to the action with the current path, what happens if we append some dots and slashes?

Navigate to the following address:

/Home/DownloadDocument?document=..\..\web.config

Do you see that this is a security risk now? Here's what just happened:

How do you make it more secure?

Most important of all: don't use the data that the user requests directly to find the files on disk. Rather have an Id (Guid) passed to a method that looks up the file name in the database and the fetches that from your disk, this way the user never, ever, gets to decide where to download the data from.

So what did we learn from this?

Don't trust the data passed to your actions. This might be truly obvious to a lot of you, but I've seen this code in production code so think twice before you introduce insecurity in your applications.

I really want to hear your thoughts on this and maybe if you have any insecurity stories of your own!

Follow @fekberg