Sanitizing string to prevent relative URI paths

Question

I have this HTTP handler I created to update information in a local SQL Express database.

I realised that it was possible for a user to use relative URI paths "/../../file.zip" as the query string, and would be able to download files outside of the restricted area.

The site is not live yet so it's not a security issue right now, but I would really like to prevent things like this.

I have added a simple string.replace line that removes any ".." from the input query.

Is there anything else that I should do here to secure this?

public void ProcessRequest(HttpContext context)
{
    string filesPath = "C:/Downloads/";
    string fileName = context.Request.QueryString["filename"];
    fileName = fileName.Replace("'", "''").Replace("..", "").Replace("/", "").Replace("\\", "");

    if (!string.IsNullOrEmpty(fileName) && File.Exists(filesPath + fileName))
    {
        context.Response.ContentType = "application/octet-stream";
        context.Response.AddHeader("Content-Disposition", string.Format("attachment; filename=\"{0}\"", fileName));
        context.Response.WriteFile(filesPath + fileName);
        //Do work to update SQL database here
    }
    else
    {
        context.Response.ContentType = "text/plain";
        context.Response.Write(filesPath + fileName + " Invalid filename");
    }
}

Answer 1

I usually use this simple code to check this problem:

(I type it directly so it may not compile, it's just to give you the idea)

private string getPath(string basePath, string fileName)
{
    var fullPath = System.IO.Path.GetFullPath(System.IO.Path.Combine(basePath, fileName));
    if (fullPath.StartsWith(basePath))
        return fullPath;
    return null;
}

The goal is to use Path.GetFullPath. This method will translate any /../ etc to a complete path. Then check that the returned path is in the allowed directory.
Be carefull that this method may returns slighty different path than expected, read MSDN for detailed explanations

Answer 2

You could have Request.QueryString["filename"] actually be a key that represents a file. The key could be a number or a random string if you don't want users to be able to easily guess file keys. You could store the mapping in a database, and use the key to retrieve the local filename (and maybe a display filename if you want to make the two differ and really hide your implementation details).