Scott Hanselman

Back to Basics: When allowing user uploads, don't allow uploads to execute code

March 27, '14 Comments [39] Posted in Tools
Sponsored By

I got an email from a reader who noticed some very odd errors happening in her web site's global error handler. Fortunately she's using ELMAH for error handling, which as you may know, is a JOY.

She was seeing:

Access to the path 'C:\Windows\security\database\secedit.sdb' is denied

Well, that's enough to make your heart skip a beat.

She looked around and found a file simply named "list.aspx" that she didn't recognize. The weird part was that this file was in the /uploads folder. That's where users can upload files with her particular CMS.

The list.aspx even has authors listed. Perhaps for their LinkedIn page?

Thanks Snailsor,FuYu,BloodSword,Cnqing,
Code by Bin
Make in China

I won't list the full list.aspx here, but rather call out some highlights of this clear malware.

It had a LOT of spaces in the opening of the file.

Meaning, they were assuming you wouldn't scroll down. Seriously. Oddly, though, it was spaces, not carriage returns. Note Line 23 never ends. It's SUPER long.

image

It pointed to a lot of (comparatively) unusual domains

It had links inside to things like

  • www.rootkit.net.cn
  • r57c99.com

Note that the second one actually serves malware and bad JavaScript, so avoid it.

It's a whole admin console for a bad guy to attack your computer

This file actually has a dropdown with "Please select a database" with values like (this is just a taste):

  • Use master dbcc addextendedproc('sp_OACreate','odsole70.dll')
  • select * from openrowset('microsoft.jet.oledb.4.0',';database=c:\windows\system32\ias\ias.mdb
  • c:\bin.asp' backup database @b to disk=@t WITH DIFFERENTIAL,FORMAT;drop table [bin_cmd];
  • Exec master.dbo.xp_cmdshell 'net user'
  • EXEC sp_configure 'xp_cmdshell'

They're going for complete control of the system, and this file is just the start.

It serves JavaScript from elsewhere

This bad aspx file also tries to bring in some bad JS from the second domain above.

That JavaScript tries to bring in even worse JavaScript from another location via an indirection. I won't even list these bits for fear that I'll get blocked for serving it!

The root of all of this is: Don't let users upload and execute code.

A fix for arbitrary code execution in user upload folders

What was the fix? Well, certainly not allowing someone to upload a file with a .aspx or .php extension for one, but also to mark the entire uploads folder as not executable! Here is the updated web.config:

<location path="upload">
<system.webServer>
<handlers accessPolicy="Read" />
</system.webServer>
</location>

I'm not a security expert, but I'd love to hear from YOU, Dear Reader, and some of the crazy stuff you've discovered on systems you manage.

About Scott

Scott Hanselman is a former professor, former Chief Architect in finance, now speaker, consultant, father, diabetic, and Microsoft employee. He is a failed stand-up comic, a cornrower, and a book author.

facebook twitter subscribe
About   Newsletter
Sponsored By
Hosting By
Dedicated Windows Server Hosting by ORCS Web
Thursday, March 27, 2014 6:04:47 AM UTC
Would a framework like MVC need similar restriction of user uploads? In theory, a View would not get executed unless a Controller approved the request first.

Our site also has user uploads. We don't place them is a sub-folder under the site, but in another folder (e.g. D:\Uploads), so the IIS doesn't see these files. Instead, we have a controller that downloads the files (/Files/Download/23).
Thursday, March 27, 2014 6:28:39 AM UTC
following the pattern convention over configuration, I would suggest a new folder app_upload, which is restriced by Default. Works quite fine for app_data.
Thursday, March 27, 2014 6:31:06 AM UTC
My new favourite since yesterday is this: search for .git folder

It is not totally dangerous but still embarrassing that one exposes the entire source code and history.
Thomas
Thursday, March 27, 2014 6:39:51 AM UTC
For me I've would block any content type except the allowed only. But I didn't knew that there is such a think that I prevent the asp.net from executing a code from a certain folder.

Thanks Scott.
Ahmed Magdy
Thursday, March 27, 2014 6:53:51 AM UTC
I've written 2 action fillers for ASP.NET MVC to allow upload special files only:
AllowUploadSpecialFilesOnlyAttribute
AllowUploadSafeFilesAttribute
VahidN
Thursday, March 27, 2014 7:10:37 AM UTC
Awesome comments so far, folks! Very useful!
Thursday, March 27, 2014 7:14:39 AM UTC
Never trust the client. Php uploaded as an image file discussed on stackoverflow.
Josh
Thursday, March 27, 2014 8:16:07 AM UTC
Like Kobi, we also place all our uploads in file locations like "C:/Uploads/Website1/..". But never knew something like this could be done so easily, thank @scott for yet another educating post. Will spread this info with others too.
Thursday, March 27, 2014 8:17:56 AM UTC
So just checking out this at the msdn, and this was introduced in IIS7.0 which means patching older sites won't be quite as easy.

But I noticed that the default for the accessPolicy is meant to be "Read", should the fix be set to "NoRemoteExecute" instead?

Just trying to understand what the differences between "Read" (default) and "Read" (explicit), if any, when running under a path.
Chris Hewitt
Thursday, March 27, 2014 8:22:23 AM UTC
I once had i client, who proudly presented his current (self-written) cms. It was in these days around the Millenium - you know, when people who could spell HTML correctly were hired as "senior developers". I was fairly when i saw that the Url looked like this:

http://myserver.com/cms/query.asp?q=SELECT%20%*%20%FROM%20%TBL_USERS&loggedin=true

When i told the client my concerns, he pointed out that with that construct, he just had to maintain one file insted of hundreds. And as IE only showed the first couple of chars of an URL without scrolling and users where "too lazy" to take a look at the complete URL, that would not be a security issue.

Ah, and for login, he cleverly constructed a ASP page, that looped through all the users and dumped out a Javascript like that:

var loggedin = /*snip: get Loggedin var from QueryString;*/

if(!loggedin){
// Redirect to LoginFrm
}

function logIn(usr, pwd)
{
if(usr =='ClientA' && pwd =='MyPassword')
{
loggedIn = true;

}
if(usr=='ClientB' && pwd =='MyOtherPwd')
{
loggedIn = true;
}

/*snip: reload referer URL w/ QS "loggedIn" var attached*/


}


Asked why this was done this way, he responded: "DB Calls are inperformant. So i make one call, and dump out all information needed"

To cut things short, he was completely prone to advice, and insisted things to be done his way. I think "clever" was the word he used.
Leo
Thursday, March 27, 2014 9:35:19 AM UTC
I'd suggest even better than restricting execute on the folder would be to save the uploaded files outside of the web root. That way they can't be browsed either - you wouldn't really want people to be able to see even legitimate uploads from other users, particularly if they need moderation first.
Andy Butland
Thursday, March 27, 2014 9:41:10 AM UTC
I once wrote a site that allowed the user to upload data and specify the file name. Each user had a separate folder.
However, I forgot to check for the special characters. So the user could give a name like "..\OtherUser\filename.ext", and it would be written to "C:\MyCoolSiteFileStorage\User1\..\OtherUser\filename.ext", which equals to "C:\MyCoolSiteFileStorage\OtherUser\filename.ext".
Several steps up would have been even worse, though the site was run as default IIS user, so System32, Program Files, etc. were not writeable.
Luckily, I noticed the bug before anyone else did.
IMil
Thursday, March 27, 2014 10:26:00 AM UTC
You can enable only static file handler in upload folder:

http://stackoverflow.com/questions/2061678/iis7-web-config-to-allow-only-static-file-handler-in-directory-uploads-of-webs
Thursday, March 27, 2014 10:41:16 AM UTC
This is another reason why you should not grant your web server process elevated permissions. When you give your IIS process higher or (please don't ever do this) admin level permissions, files can be written anywhere on disk and executed from anywhere on disk.

Only grant security permissions that are needed, especially to internet facing services. Execute permissions should be treated with extreme care.
Thursday, March 27, 2014 10:45:15 AM UTC
A long time ago I checked the content of an upload folder for an intranet application that contained files for a file attachment feature. It contained everything from blurry 2-second videos to Window shortcuts. The allowed file types were restricted prompty after we saw that!
Thursday, March 27, 2014 10:49:40 AM UTC
I know its not popular, but you could store the files as blobs in a database where they have no chance of being executed. It will be a little less performant, but it will be a lot easier to ensure you have it backed up properly. Make a page that streams out files based on the id of the file. You can also do much better management of permissions this way, and I'm not aware of a way to execute a blob in the DB that doesn't involve some kind of direct, non-web access(like ssh) to the server.
Thursday, March 27, 2014 12:29:45 PM UTC
i had borrowed a co-worker's laptop while traveling a few years ago and the laptop would misbehave occasionally. i'd get these weird looking ads in the bottom-right corner of any browser :(

It turned out the hosts file was edited but it took me forever to figure that out because of the huge amount of whitespace before the very bad lines at the end of the file. At first glance, everything looked fine! At that point i learned to look at the scrollbars when searching for funny business on a machine.
Thursday, March 27, 2014 12:55:51 PM UTC
It's so easy to forget the basics - thanks for raising the awareness. Seeing OWASP and other security measures - anti-forgery tokens, injection proofing often the items in your post are overlooked. Such a simple fix to prevent such a dangerous attack - it's obvious and that's why it gets missed.

Dangerous code shown in the example can have a major impact including browser blocking (FireFox's "Reported Attack Site" and blacklisting by search engines. This snippet is worth real $$$ when it impacts such an attack has on the business side of things such as lost revenue.

I used to fight a PHP site running legacy code not written by myself with issues like this all the time. Management couldn't justify the cost of an upgrade or patching the code until I pointed out that we lost $XXXXX a day each time the attack was done because customers could not use the site to purchase the product. This made it real easy to justify a $2K upgrade.

Thursday, March 27, 2014 12:56:58 PM UTC
Best policy is to *never* store user uploaded content into an IIS content folder. Store it somewhere else where no direct URL can get at it, whenever possible. (it does mean you'll need to build a handler to serve it, but that's a small price to pay for security). That's just my humble opinion...
Thursday, March 27, 2014 1:47:38 PM UTC
@Tomas: that is true when you are coding the CMS , but what if you are using others CMS , then adding policy to that folder will be the best choice .
Sam
Thursday, March 27, 2014 1:58:35 PM UTC
I've seen this quite a bit with poorly setup Wordpress sites. I got an email from Amazon AWS saying that one customers Wordpress blog was serving up a phishing site. I took it offline and went though all the rubbish that had been uploaded and it was certainly an eye-opener. There was a single php file which gave a whole admin console with numerous 'tools' which you could run on the server. It even had an option to try all the exploits and return a list of the successful ones!
Tim
Thursday, March 27, 2014 2:06:57 PM UTC
how do you restrict when you are uploading files to sharepoint from .net ?
panchi
Thursday, March 27, 2014 2:13:03 PM UTC
Its always better to block the user from uploading everything and white listing whats allowed instead of trying to blacklist certain extensions.
Justen Martin
Thursday, March 27, 2014 2:14:38 PM UTC
Also, a lot of time people will hide malicious code in a legitimate file, but hide it by creating lots of horizontal space through spaces. This way any casual glance won't show any odd code in the file.
Justen Martin
Thursday, March 27, 2014 2:21:35 PM UTC
Maybe the developer was trying to re-invent WebDAV?

Seriously though, improper upload handling, SQL injection, XSS, these are all things that have been around since the dawn of the web. I guess they'll be here until the dusk.
Thursday, March 27, 2014 4:35:34 PM UTC
An application serving documents.

The access to confidential documents were filter by this querystring parameter:

confidential=true

Anyway, when travelling on the help pages of this web application we were able to find all server names with their root passwords!!!????????



The security page in the doc just said this single line: "the application is protected by Unix".



Security is not a concern for some...









Fred
Thursday, March 27, 2014 5:56:46 PM UTC
@Sam: True, but you'd be surprised how many enterprise applications accept user uploads, store them on temp folders for processing, and turns out the temp folders are URL accessible :/

So it's definitely not only for CMS you should be careful!
Thursday, March 27, 2014 8:58:55 PM UTC
Here's a good gotcha... We had code in place to ensure only file names ending in 'jpeg' or 'jpg' could be uploaded. Unfortunately IIS will process a request for a file with an extension of '.asp;jpg' as a classic ASP request, and execute the file as a script. Not checking where the dot was turned out to be a very time consuming mistake.

We adopted the web.config trick as a second line of defence. It seems to work well even for classic ASP sites.
Darren
Saturday, March 29, 2014 7:04:37 PM UTC
The update for the web.config is not enough because the uploader can upload a web.config in the upload folder and overwrite the handler configuration so I think the web.config should be updated this way:

<location path="upload" allowOverride="false">
<system.webServer>
<handlers accessPolicy="Read" />
</system.webServer>
</location>

This will prevent the sub folders to override the handler.
Amir Jahangard
Sunday, March 30, 2014 10:33:34 AM UTC
I realize it's a CMS so this doesn't completely apply, but don't store the same file name on disk.

Upload types should be whitelisted as well. Media type testing can be a good strategy as well. Is the .png really an image file? If not don't allow the upload.
Phil DeVeau
Monday, March 31, 2014 3:29:49 PM UTC
Great back to basics tip. A great reminder for general developers to stay security conscious. There are varying degrees of infosec need and ability but every developer can benefit from a reminder to think critically and to question all aspects of an application.
Monday, March 31, 2014 10:48:13 PM UTC
Not all soccer formations have a sweeper, but when the position is incorporated it is the most
essential defensive function, aside from the goalkeeper's.
The sweeper is in charge of the defensive group and backs up the
other a few defenders. He gets to be the final line of protection before the goalkeeper.
A sweeper demands to be quickly on her ft
and sharp mentally, observing the entire field of play to foresee prospective defensive breakdowns.
Endurance is necessary also, as he will possibly have to operate for most of the match, constantly filling defensive gaps
and relocating the ball forward.

FIFA World Rankings as of Sept. 12 Rank Crew Adjust one Spain - 2 Argentina Up 2 three Germany Down 1 4 Italy Up 2 five Colombia Down two six Belgium
Up 4 seven Uruguay Up five eight Brazil Up 1 9
Netherlands Down 4 10 Croatia Down two 11 Portgual Down 4 12 Greece Down one thirteen United States Up six 14 Switzerland Up one fifteen Russia Up one 16 Chile Up five seventeen England Down 3 eighteen Bosnia-Herzegovina Down 5 19 Ivory Coast Down
one 20 Ecuador Down three
Tuesday, April 01, 2014 5:52:57 AM UTC
If you must store the file in a web accessible folder, generate random file names without extension (and serve them through a handler. store meta data in db).
This prevents the user being able to browse to the file he has uploaded, and prevents hacks in the file name.

Storing in a blob or web inaccessible folder is a better approach.
Eilev Hagen
Tuesday, April 01, 2014 5:59:51 PM UTC

dim input = txtusername.text

cmdSQL = "SELECT * FROM user_table WHERE username=" + txtusername.text
Tom
Thursday, April 03, 2014 5:46:37 PM UTC
Ҭhanks for sharing such a pleasant idea, article is nice, thats why i have rad it entirely

Feel free to visit mmy blog post ... fixed
pitch rc helicopter (rcgalore.net)
Monday, April 07, 2014 4:31:56 PM UTC
I don't usually see a need to store user data in a web-accessible folder, but if you do (eg for forum avatar images) just create a folder called Uploads inside the App_Data folder. It will inherit the security restrictions IIS already provides for the App_Data folder. You can still read files from this folder in C# land but you can only serve them through IIS by creating a simple HTTP handler. You should *also* limit the uploader to only save files of the extension you are expecting (eg .jpg/.jpeg). Any HTTP handler you create to serve files from non-web folders should be hard coded to only ever serve from a whitelisted set of extension (ie only opt-IN image extensions rather than trying to opt out (or block) .aspx etc).

Often this approach is easier than trying to work with an external folder (esp in some 3rd party hosting environment).
Wednesday, April 09, 2014 8:59:26 AM UTC
If we set the accessPolicy to "Read" in the upload folder then how come the uploader still works? Doesn't it need the Write permission too to, well, write in the folder?
Anthony
Tuesday, April 15, 2014 8:33:08 PM UTC
Never allow uploads to the same domain, even outside of the web root! Different TLD means no access to site cookies (so no session hijacking), no execution rights, no handlers - if set up properly (also improves hosting performance). User-provided files must be presumed malicious. No file type is safe. Start exploring the subject by searching the internet for "GIFAR".
Endrju
Wednesday, May 21, 2014 2:15:35 PM UTC
Actually it looks like the upload folder is ASP.NET application subfolder which is bad in first play for upload files. Now she has to remember to update this setting each time location changes. Not to mention it's blacklisting. We should always do whitelisting...
Artur Nowocin
Comments are closed.

Disclaimer: The opinions expressed herein are my own personal opinions and do not represent my employer's view in any way.