Catch the security flaw #6

If you can find the security issue with this piece of code, write about it by adding a comment to this blog post.

This is the scenario:-

1. There is a Web site that allows end users to upload their pictures.

2. On the Web server, the Web site is physically located at C:\Inetpub\wwwroot\sampleapp, which is obviously a virtual directory.

3. The uploaded pictures are stored on the file system in the following directory C:\Uploads\{username}. This is not a virtual directory. {username} is the logged in user’s username and can contain only alphabets and numbers.

This is what the page looks like to the end user.

This is the code that gets executed. The code for the UploadPicture.aspx.cs file is this.

    1: public partial class UploadPicture : System.Web.UI.Page

    2: {

    3:     protected void Page_Load(object sender, EventArgs e)

    4:     {

    5:         lblUserName.Text = "Welcome " + User.Identity.Name;

    6:     }

    7:     protected void btnSubmit_Click(object sender, EventArgs e)

    8:     {

    9:         new UploadHelper().UploadFile(fileUploadControl, User.Identity.Name);

   10:     }

   11: }

The code for the UploadHelper.cs file is this:-

    1: public class UploadHelper

    2: {

    3:     private string uploadFolder;

    4:     public UploadHelper()

    5:     {

    6:         uploadFolder = @"C:\Uploads\";

    7:     }

    8:     

    9:     private void SetUpUploadFolder(string userName)

   10:     {

   11:         uploadFolder += userName + @"\";

   12:         if (!System.IO.Directory.Exists(uploadFolder))

   13:             System.IO.Directory.CreateDirectory(uploadFolder);

   14:     }

   15:     

   16:     public string UploadFile(FileUpload fp, string userName)

   17:     {

   18:         string extension = fp.PostedFile.FileName.Substring(fp.PostedFile.FileName.LastIndexOf("."));

   19:         if (extension.ToLower() == ".jpg" || extension.ToLower() == ".gif")

   20:         {

   21:             string strFileName = fp.PostedFile.FileName.Substring(fp.PostedFile.FileName.LastIndexOf("\\") + 1);

   22:             SetUpUploadFolder(userName);

   23:             string strFullPath = uploadFolder + strFileName;

   24:             fp.SaveAs(strFullPath);

   25:             return strFileName;

   26:         }

   27:         return null;

   28:     }

   29: }

If I login to the Web site as “varun” and upload a picture with name mypic.gif, it gets uploaded to this folder on the Web server:-

C:\Uploads\varun\mypic.gif

Happy Hunting!

Comments

• Anonymous
  April 10, 2009
  PingBack from http://asp-net-hosting.simplynetdev.com/catch-the-security-flaw-6/

• Anonymous
  April 10, 2009
  Bad guy uploads a file named "evil.htm.gif" and it's saved as "evil.htm".  Bad guy now has script-injection from a page served by goodguy.com. Bad guy uploads a file named "evil.aspx.jpg" and it's saved as "evil.aspx".  Depending on permissions, bad guy may have RCE on the server. Bad guy uploads file with name like "evil....windowssystem32trojan.exe.gif" and depending on permissions, bad guy may be able to drop a file in the system folder.

• Anonymous
  April 10, 2009
  The .net framework file functions allow "/" as a directory separator also, so you could save a file with a name of "../some_other_user/some_image.jpg" and overwrite another user's images. To avoid this, you should be using the static methods on System.IO.Path (GetExtension and GetFileName) to get the parts of the file name. Personally, I'd also use Path.Combine instead of string concatenation but in this case it's mostly an aesthetic difference.

• Anonymous
  April 10, 2009
  The comment has been removed

• Anonymous
  April 10, 2009
  Thanks JD! You are absolutely right! There are a few other attack vectors though, which may be possible. I will blog about those in a couple of weeks.

• Anonymous
  August 04, 2009
  The issue is that the username is not being validated here. One may pass a user name in this fashion "..sam1" and now you would have created the folder sam1 in the C drive root. One could hog up the diskspace on the server by first creating the folders as reqd and then upload all sorts of files on the server. Also to hide code within the jpg , double extensions is not the only trick to use. There are multiple ways to hide the code within a jpg file without even having to change the extension.

• Anonymous
  January 14, 2010
  The comment has been removed