Any security issues using GetFiles(); on eCom product?

Jacob Storgaard Jensen

Posted on 03/04/2017 10:37:59

Hi guys, I'm planning on using this code to loop out product related files on a ecom product template - is there any security issues using GetFiles that way?

@using System.Web;
@using System.IO;
@using System.Linq;

@{
var productNumber = "";
productNumber = GetString("Ecom:Product.Number");

    var root = "/Files/Images/ecom/products/relatedfiles";
    var thedirectory = new DirectoryInfo(HttpContext.Current.Server.MapPath("~"+root+"/"+productNumber));
    if (thedirectory.Exists) {
        var files = thedirectory.GetFiles();
        if (files.Any()) {
            foreach (var file in files) {
                var fileName = file.Name;
                var fileSize = CustomMethods.SizeSuffix(file.Length, 2);
                var fileUrl = "Admin/Public/DWSDownload.aspx?File=" + string.Format("{0}/{1}", root, fileName);
                <pre><a href="@fileUrl">@fileName @fileSize</a></pre>
            }
        }
    } else {
    <pre>nodir</pre>
    }

}

Replies

Nicolai Pedersen

Posted on 03/04/2017 13:51:20

Hi Jacob

It looks ok - since you do not get data from post, querystring etc.

If you have a lot of load on the product detail, IO could become an issue, but that would be a LOT of load.

BR Nicolai

Jacob Storgaard Jensen

Posted on 03/04/2017 14:50:00

So if I were to change:

productId = GetString("Ecom:Product.Number");

var request = HttpContext.Current.Request;
productId = request.QueryString["ProductNo"];

This would expose an insecurity?
It's supposed to be used to retrieve files for a specific product from a product list view via ajax. To avoid all products in the list loading their respective files on load.
Will the querystring allow for ../ traversing back through to other files/folders than the ones in the "/Files/Images/ecom/products/large/"?

Nicolai Pedersen

Posted on 03/04/2017 15:30:20

Yes, that could be problematic code. Allowing ../ is also even more problematic.

Jacob Storgaard Jensen

Posted on 03/04/2017 15:41:44

Wasn't planning on using ../ was just me thinking if that was why it was problematic... :-)

Is it problematic because you can bombarde the server with requests via the querystring or why is it problematic? Because it wouldn't render any files if the folder didn't exsist... because I'm only defining the productnumber part of the path via the querystring...

Nicolai Pedersen

Posted on 03/04/2017 15:51:15

Hi Jacob

You could get access to any file on the server if you allow something like that (depending on server secuiryt settings)...

Could i.e. easily get hold of your config file with database user etc.

So be very careful if you allow querystring/post/session/cookie information the mappath statement.

BR Nicolai

Jacob Storgaard Jensen

Posted on 03/04/2017 16:05:38

Sorry to ask so many questions on this: But also in the case where the directory i'm requesting is constructed the way I do it?

First the root is defined and hardcoded:

var root = "/Files/Images/ecom/products/relatedfiles";

then the productnumber is added from the querystring:

var thedirectory = new DirectoryInfo(HttpContext.Current.Server.MapPath("~"+root+"/"+productNumber));

Could this really traverse up the folder structure by using ../
e.g.: /Files/Images/ecom/products/relatedfiles/../../../somefile.xxx

What if I add a check to see if the querystring value contains .. or ~ then do nothing...

Nicolai Pedersen

Posted on 03/04/2017 22:01:41

This post has been marked as an answer

Yes, that would make it possible to traverse your folder structure.

So you need to make sure unwanted and invalid characters is not in the querystring.

Then when mapped, make sure the mapped folder is below the root folder.

i.e.

If HttpContext.Current.Server.MapPath("~"+root+"/"+productNumber).StartsWith(HttpContext.Current.Server.MapPath("~"+root+"/"))

or something like that!

Votes for this answer: 1

You must be logged in to post in the forum

Developer forum

Any security issues using GetFiles(); on eCom product?

Replies