StackOverflow2013

Note that there are some explanatory texts on larger screens.

plurals

PO
text
Body
copied!<h1>TL;DR</h1> <p><strong>Stop.</strong> Shelve your script <strong>right now.</strong> It is a gaping security hole just <em>waiting</em> to be exploited. Read the following resources:</p> <ul> <li><a href="http://perldoc.perl.org/perlsec.html" rel="nofollow">perlsec</a></li> <li>the <a href="https://www.securecoding.cert.org/confluence/displa/perl/CERT+Perl+Secure+Coding+Standard" rel="nofollow">CERT Perl Secure Coding Standard</a> (particularly the section on <a href="https://www.securecoding.cert.org/confluence/display/perl/01.+Input+Validation+and+Data+Sanitization" rel="nofollow">Input Validation and Data Sanitization</a>)</li> <li>the OWASP page on <a href="https://www.owasp.org/index.php/Unrestricted_File_Upload" rel="nofollow">Unrestricted File Upload</a></li> <li>the InfoSec page on <a href="http://resources.infosecinstitute.com/file-upload-vulnerabilities/" rel="nofollow">Complete File Upload Vulnerabilities</a></li> <li>the CWE page on <a href="http://cwe.mitre.org/data/definitions/434.html" rel="nofollow">Unrestricted Upload of File with Dangerous Type</a></li> <li>the SANS recommendations for <a href="http://software-security.sans.org/blog/2009/12/28/8-basic-rules-to-implement-secure-file-uploads" rel="nofollow">8 Basic Rules to Implement Secure File Uploads</a>.</li> </ul> <p>When you have read--and understood--all of them, stop and think if you <em>really</em> need to let users upload files onto your server. Think <strong>long</strong> and <strong>hard.</strong> Can you really account for all of the listed vulnerabilities? If you still feel like you need to do this, consider enlisting the help of a security expert. Follow the guidelines laid out in the above resources <em>carefully</em> and understand that a mistake in your design could compromise your entire site.</p> <hr> <p>I understand that this is just a test script, not a production application (at least, I really hope that's the case), but even so, what you are doing (and particularly <em>how</em> you are doing it) is a very, <em>very</em> <strong>bad idea.</strong> Here are a select <em>few</em> of the reasons why, from OWASP's page on <a href="https://www.owasp.org/index.php/Unrestricted_File_Upload" rel="nofollow">Unrestricted File Upload</a>:</p> <ul> <li>The website can be defaced.</li> <li>The web server can be compromised by uploading and executing a web-shell which can: run a command, browse the system files, browse the local resources, attack to other servers, and exploit the local vulnerabilities, and so on.</li> <li>This vulnerability can make the website vulnerable to some other types of attacks such as XSS.</li> <li>Local file inclusion vulnerabilities can be exploited by uploading a malicious file into the server.</li> </ul> <p>More from OWASP:</p> <blockquote> <p>Uploaded files represent a significant risk to applications. The first step in many attacks is to get some code to the system to be attacked. Then the attack only needs to find a way to get the code executed. Using a file upload helps the attacker accomplish the first step.</p> <p>The consequences of unrestricted file upload can vary, including complete system takeover, an overloaded file system, forwarding attacks to backend systems, and simple defacement.</p> </blockquote> <p>Pretty scary stuff, huh?</p> <h1>The problems</h1> <h2>Your code</h2> <p>Let's start by looking at some of the problems with the code you posted.</p> <h3>No strict, no warnings</h3> <p>Start putting <code>use strict; use warnings;</code> at the top of <em>every Perl script you ever write.</em> I recently had the pleasure of fixing a CGI script that contained a snippet something like this:</p> <pre><code>my ($match) = grep { /$usrname/ } @users; </code></pre> <p>This code was used to check that the username entered in an HTML form matched a list of valid users. One problem: the variable <code>$usrname</code> was misspelled (it should have been <code>$username</code> with an 'e'). Since strict checking was off, Perl happily inserted the value of the (undeclared) global variable <code>$usrname</code>, or <code>undef</code>. That turned the innocent-looking snippet into this monstrosity:</p> <pre><code>my ($match) = grep { // } @users; </code></pre> <p>which matches <strong>everything</strong> in the valid users list and returns the first match. You could enter anything you wanted into the username field in the form and the script would think you were a valid user. Since warnings were also off, this was never caught during the development process. When you turn warnings on, the script will still run and return a user, but you also get something like this:</p> <pre><code>Name "main::usrname" used only once: possible typo at -e line 1. Use of uninitialized value $usrname in regexp compilation at -e line 1. </code></pre> <p>When you also turn on strict, the script fails to compile and won't even run at all. There are other problems with this snippet (for example, the string 'a' will match the username 'janedoe'), but strict and warnings at least alerted us to one major issue. I cannot stress this enough: always, <em>always</em> <code>use strict; use warnings;</code></p> <h3>No taint mode</h3> <p>The first rule of web development is, "Always sanitize user input." Repeat after me: Always sanitize user input. One more time: <strong>Always sanitize user input.</strong> In other words, never blindly trust user input without validating it first. Users (even those that are not malicious) are very good at entering creative values into form fields that can break your application (or worse). If you don't restrict their creativity, there is no limit to the damage a malicious user can do to your site (refer to the <a href="https://www.owasp.org/index.php/Top_10_2013-A1-Injection" rel="nofollow">perennial #1 vulnerability on the OWASP Top 10, injection</a>).</p> <p>Perl's taint mode can help with this. Taint mode forces you to check all user input before using it in certain potentially dangerous operations like the <code>system()</code> function. Taint mode is like the safety on a gun: it can prevent a lot of painful accidents (although if you really want to shoot yourself in the foot, you can always turn off the safety, like when you untaint a variable without actually removing dangerous characters). Turn on taint mode in <em>every CGI script you ever write.</em> You can enable it by passing the <code>-T</code> flag, like this:</p> <pre><code>#!/usr/bin/perl -T </code></pre> <p>Once taint mode is enabled, your script will throw a fatal error if you try to use tainted data in dangerous situations. Here's an example of such a dangerous situation that I found in a random script on the internet:</p> <pre><code>open(LOCAL, ">/home/Desktop/$name") or die $!; </code></pre> <p>Ok, I lied, that snippet isn't from a random script, it's from <em>your</em> code. In isolation, this snippet is just begging to be hit with a <a href="http://en.wikipedia.org/wiki/Directory_traversal_attack" rel="nofollow">directory traversal attack</a>, where a malicious user enters a relative path in order to access a file that they shouldn't have access to.</p> <p>Fortunately, you've done something right here: you ensured that <code>$name</code> will contain no directory separators by using a regex<sup>*</sup>. This is exactly what taint mode would require you to do. The benefit of taint mode is that if you forget to sanitize your input, you will be alerted immediately with an error like this:</p> <pre><code>Insecure dependency in open while running with -T switch at foo.cgi line 5 </code></pre> <p>Like strict, taint mode forces you to address problems in your code immediately by causing the program to fail, instead of allowing it to quietly limp along.</p> <p>* You did some<em>thing</em> right, but you also did some <em>things</em> wrong:</p> <ul> <li>Your program will die if the user passes in only a filename with no directory separators, e.g. <code>foo</code></li> <li>You don't remove special characters that could be interpreted by a shell, like <code>|</code></li> <li>You never sanitize the variable <code>$file</code> and yet you try to use it to read a file later in your code</li> <li>You don't check if the file you're writing to already exists (see "No check for file existence" below)</li> <li>You allow the user to choose the name of the file that will be stored on your server, which gives them far more control than you should be comfortable with (see "Allowing the user to set the file name" below)</li> </ul> <h3>CGI::Carp fatalsToBrowser</h3> <p>I'll give you the benefit of the doubt on this one since you're still testing your script, but just in case you weren't aware and since I'm already talking about CGI security issues, <strong>never enable CGI::Carp's fatalsToBrowser option in a production environment.</strong> It can reveal intimate details about the inner workings of your script to attackers.</p> <h3>Two-argument <code>open()</code> and global filehandles</h3> <p>Two-argument <code>open()</code>, e.g.</p> <pre><code>open FH, ">$file" </code></pre> <p>has a <a href="https://www.securecoding.cert.org/confluence/pages/viewpage.action?pageId=76775519" rel="nofollow">host of security risks associated with it</a> when users are allowed to specify the file path. Your script mitigates many of these by using a hard-coded directory prefix, but that in no way diminishes the fact that using two-argument open can be <strong>very dangerous.</strong> In general, you should use the three-argument form:</p> <pre><code>open my $fh, ">", $file </code></pre> <p>(which is still plenty dangerous if you allow the user to specify the file name; see "Allowing the user to set the file name" below).</p> <p>Also note that instead of the global filehandle <code>FH</code> I switched to a lexical filehandle <code>$fh</code>. See CERT's page <a href="https://www.securecoding.cert.org/confluence/display/perl/FIO00-PL.+Do+not+use+bareword+file+handles" rel="nofollow">Do not use bareword filehandles</a> for some reasons why.</p> <h3>No check for file existence</h3> <p>You don't check whether a file already exists at <code>/home/Desktop/$name</code> when you open it for writing. If the file already exists, you will truncate it (erase its contents) <em>as soon as the <code>open()</code> call succeeds,</em> even if you never write anything to the file. Users (malicious and otherwise) are likely to clobber each other's files, which doesn't make for a very happy user base.</p> <h3>No limit on file size</h3> <p>"But wait," you say, "I set <code>MAX_FILE_SIZE</code> in my HTML form!" Understand that this is merely a suggestion to the browser; attackers can easily edit HTTP requests to remove this condition. <strong>Never rely on hidden HTML fields for security.</strong> Hidden fields are plainly visible in the HTML source of your page and in the raw HTTP requests. You must limit the maximum request size on the <em>server side</em> to prevent users from loading massive files to your server and to help alleviate one type of denial of service attack. Set the <code>$CGI::POST_MAX</code> variable at the beginning of your CGI script like this:</p> <pre><code>$CGI::POST_MAX=1024 * 30; # 30KB </code></pre> <p>Or even better, find CGI.pm on your system and change the value of <code>$POST_MAX</code> to set it globally for all scripts that use the CGI module. That way you don't have to remember to set the variable at the beginning of every CGI script you write.</p> <h3>CGI doesn't match the HTML form</h3> <p>The POST variable you use for the file path in your HTML form, <code>userfile</code>, does not match the variable you look for in your CGI script, <code>file</code>. This is why your script is failing with the error</p> <pre><code>Is a directory </code></pre> <p>The value of</p> <pre><code>$cgi->param('file') </code></pre> <p>is <code>undef</code> so your script tries to open the path</p> <pre><code>/home/Desktop/ </code></pre> <p>as a regular file.</p> <h3>Obsolete method for handling upload</h3> <p>You are using the old (and obsolete) method of handling uploads with CGI.pm where <code>param()</code> is used to get both the file name and a lightweight filehandle. This will not work with strict and is insecure. The <code>upload()</code> method was added in v2.47 (all the way back in 1999!) as a preferred replacement. Use it like this (straight out of the <a href="http://perldoc.perl.org/CGI.html#PROCESSING-A-FILE-UPLOAD-FIELD" rel="nofollow">documentation for CGI.pm</a>):</p> <pre><code>$lightweight_fh = $q->upload('field_name'); # undef may be returned if it's not a valid file handle if (defined $lightweight_fh) { # Upgrade the handle to one compatible with IO::Handle: my $io_handle = $lightweight_fh->handle; open (OUTFILE,'>>','/usr/local/web/users/feedback'); while ($bytesread = $io_handle->read($buffer,1024)) { print OUTFILE $buffer; } } </code></pre> <p>where <code>field_name</code> is the name of the POST variable that holds the file name (in your case, <code>userfile</code>). Notice that the sample code does not set the output filename based on user input, which leads to my next point.</p> <h3>Allowing the user to set the file name</h3> <p><strong>Never</strong> allow users to choose the file name that will be used on your server. If an attacker can upload a malicious file to a known location, it becomes significantly easier for them to exploit. Instead, generate a new, unique (to prevent clobbering), difficult-to-guess file name, preferably in a path outside your web root so users cannot access them directly with a URL.</p> <h2>Other issues</h2> <p>You haven't even <em>begun</em> to address the following issues.</p> <h3>Authentication</h3> <p>Who is allowed to upload files using your web app? How will you ensure that only authorized users are uploading files?</p> <h3>Access control</h3> <p>Are users allowed to see the files uploaded by other users? Depending on the file content, there could be major privacy issues at stake.</p> <h3>Number and rate of uploads</h3> <p>How many files is one user allowed to upload? How many files is a user allowed to upload in a fixed period of time? If you don't restrict these, one user could easily eat up all of your server resources very quickly, even if you enforce a maximum file size.</p> <h3>Dangerous file types</h3> <p>How will you check that users are not uploading dangerous content (for example, executable PHP code) to your server? Simply checking the file extension or content type header is not enough; attackers have found some very creative methods for circumventing such checks.</p> <h1>"But, but, I'm only running this on my corporate intranet..."</h1> <p>You may be tempted to disregard these security issues if your script is not accessible from the internet. However, you still need to consider</p> <ul> <li>In-office pranksters</li> <li>Disgruntled coworkers</li> <li>Collaborators and outside contractors who either need access to your app or who shouldn't have access</li> <li>Managers who love your app so much that they decide to open it up to users on the internet without your knowledge, possibly after you've transferred to another group or left the company</li> </ul> <h1>"What should I do?"</h1> <p>Scrap your existing code. Read the resources I listed in the first paragraph, carefully. Here they are again:</p> <ul> <li><a href="http://perldoc.perl.org/perlsec.html" rel="nofollow">perlsec</a></li> <li>the <a href="https://www.securecoding.cert.org/confluence/display/perl/CERT+Perl+Secure+Coding+Standard" rel="nofollow">CERT Perl Secure Coding Standard</a> (particularly the section on <a href="https://www.securecoding.cert.org/confluence/display/perl/01.+Input+Validation+and+Data+Sanitization" rel="nofollow">Input Validation and Data Sanitization</a>)</li> <li>OWASP's <a href="https://www.owasp.org/index.php/Unrestricted_File_Upload" rel="nofollow">Unrestricted File Upload</a></li> <li>InfoSec's <a href="http://resources.infosecinstitute.com/file-upload-vulnerabilities/" rel="nofollow">Complete File Upload Vulnerabilities</a></li> <li>CWE's <a href="http://cwe.mitre.org/data/definitions/434.html" rel="nofollow">Unrestricted Upload of File with Dangerous Type</a></li> <li>SANS recommendations for <a href="http://software-security.sans.org/blog/2009/12/28/8-basic-rules-to-implement-secure-file-uploads" rel="nofollow">8 Basic Rules to Implement Secure File Uploads</a></li> </ul> <p>Consider carefully if you <em>really</em> need to do this. If you just need to give users a place to store files, consider using (S)FTP instead. This would certainly not eliminate all of the security risks, but it would eliminate a big one: your custom CGI code.</p> <p>If after careful consideration you still think this is necessary, work through some <a href="http://perl-tutorial.org/#index3h1" rel="nofollow"><em>recent</em> Perl tutorials</a> to make sure you can use and understand modern Perl programming conventions. Instead of CGI.pm, use a framework like <a href="http://www.catalystframework.org/" rel="nofollow">Catalyst</a>, <a href="http://perldancer.org/" rel="nofollow">Dancer</a>, or <a href="http://mojolicio.us/" rel="nofollow">Mojolicious</a>, all of which have plugins that can handle tricky areas like user authentication and sessions so you don't have to re-invent the wheel (poorly).</p> <p>Follow all of the security guidelines listed in the above resources and consider enlisting the help of an expert in web security. Tread carefully: a single mistake in your code could allow an attacker to compromise your entire site and possibly even other machines on your network. Depending on what country your company and your users are in, this could even have legal ramifications.</p> <p></soapbox></p>

Querying!

Guidance

An individual column

Larger individual text columns get their own page to allow for proper reading.

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload