Page 1 of 1

Check "HTTP_RAW_POST_DATA" type before writing the file ?

Posted: Wed Jul 08, 2009 11:21 am
by Horous
Hi everyone !

I spent the day making searches on the web, but I can't find what I'm looking for.
I guess there will be someone here that will be able to help me on this...

So I've got a Flash application wich allows the user to draw whatever they want to and send me the resulting image.
I use the JPGEncoder class in ActionScript 3, it generates a ByteArray, then I call a PHP script to handle the file creation.
Here's a part of the AS3 code :

Code: Select all

 
var _JPGByteArray:ByteArray = _JPGEncoder.encode( _finalBitmapData );
 
var _saveURLRequestHeader:URLRequestHeader = new URLRequestHeader( "Content-type", "application/octet-stream" );
var _saveURLRequest:URLRequest = new URLRequest( "save_jpg.php?name=" + _finalName );
_saveURLRequest.requestHeaders.push( _saveURLRequestHeader );
_saveURLRequest.method = URLRequestMethod.POST;
_saveURLRequest.data = _JPGByteArray;
 
var _saveURLLoader:URLLoader = new URLLoader();
_saveURLLoader.dataFormat = URLLoaderDataFormat.BINARY;
_saveURLLoader.addEventListener( Event.COMPLETE, _saveComplete );
_saveURLLoader.load( _saveURLRequest );
 
And here's the PHP code I use to create the file on the server :

Code: Select all

<?php
if( isset( $GLOBALS[ "HTTP_RAW_POST_DATA" ] ) )
{
    $jpg = $GLOBALS[ "HTTP_RAW_POST_DATA" ];
 
    if( $fp = fopen( "drawings/".$_GET[ 'name' ], 'w' ) )
    {
        if( fwrite( $fp, $jpg ) )
        {
            if( fclose( $fp ) )
            {
                echo 'ok';
            }
            else
            {
                echo 'fclose_failed';
            }
        }
        else
        {
            echo 'fwrite_failed';
        }
    }
    else
    {
        echo 'fopen_failed';
    }
}
else
{
    echo 'process_failed';
}
?> 
But of course I can't use this safely, for I guess anyone could send any raw data to the PHP script and make it write a corrupted file on the server.

So my first question is :
Is there a way to perform a type check on HTTP_RAW_POST_DATA to make sure it actually is a jpg image file ?
Or is there a way (other than HTTP_RAW_POST_DATA) to get the data that will allow this kind of check ?

If not :
Could I avoid writing the file on the server and send it directly to me attached to an email ?
But would it be safer ?

Thank you for your help !

Re: Check "HTTP_RAW_POST_DATA" type before writing the file ?

Posted: Wed Jul 08, 2009 3:56 pm
by kaisellgren
You are right. Anyone can send arbitrary POST data to your script.
Is there a way to perform a type check on HTTP_RAW_POST_DATA to make sure it actually is a jpg image file ?
You could try resizing the submitted data, but that does not guarantee that it's only a valid image. Combining multiple files is possible in several ways. For example, Javascript and JPEG. However, this does not matter that much. As long as you handle the files as images, you are okay. That is, you need to serve the uploaded files through a PHP script (or serve them through a separate domain). Take a look at this:

Code: Select all

$data = file_get_contents('/home/account/files/file.png');
header('Content-Type: image/png');
header('Content-Length: '.strlen($data));
header('X-Content-Type-Options: nosniff');
echo $data;
No matter what does the file contain, the client will handle it as a PNG image. The X-Content-Type-Options is for IE only (it sometimes sniffs file contents and changes the content type even if you specify the Content-Type header). You should also put the "uploaded" file outside your document root, otherwise, someone can access it directly, which should never be possible for user uploaded files.

You have other worse problems in your implementation.

Code: Select all

if( $fp = fopen( "drawings/".$_GET[ 'name' ], 'w' ) )
User supplied data may never be trusted. Simple way to exploit this vulnerability, which is called an LFI (Local File Inclu(de/sion)), is to do the following:

Code: Select all

?name=../index.php
and overwrite your index.php with a backdoor, for instance. One way to fix this would be to first use basename() to get the filename part of the name, and then apply a validation (or you could filter/sanitize it with a strict whitelist):

Code: Select all

$name = basename($_GET['name']);
if (!preg_match("/^[^a-z0-9_ -.]+$/iD",$name)) // add more characters *if* needed
 die('Nuh uh');
if( $fp = fopen( "drawings/".$name, 'w' ) )
Under normal circumstances the check would succeed. In case someone tries to do something nasty, you would tell him that the name was improper or what ever you feel like as long as you do not accept improper data.

Re: Check "HTTP_RAW_POST_DATA" type before writing the file ?

Posted: Thu Jul 09, 2009 3:25 am
by Horous
Ok, first I want to thank you for your quick and detailed answer.

I think I understand the handling of the data as image with file_get_contents() and the headers.
That will be helpful for displaying the images on a page.

I also get the backdoor thing, I never thought about overwriting the index.php, that's a huge leak indeed...
I'll use basename() to prevent file creating in any other directory than the one I want.

I've never used regexp though, so I read some tutorials but still I'm not sure I get everything (I guess it takes a bit more time to get used to it) :
- why the '^' inside the brackets before the 'a' ? Is it to make sure there is nothing before the authorized characters ?
- why the '+' at the end ? Is this a way to make sure there is at least one character in the string ?
- if the 'i' at the end stands for 'case insensitive', what does the 'D' stand for ?

And won't that still allow a user to create a 'file.php' ?
Even if I can now control where it is created, it would not be very hard to find that folder and execute the corrupted file...
Do I need to check the file extension as well ? Would it be efficient, as I guess it is possible to put a php script in a 'file.jpg' ?
Or may I use the header() method you showed me to treat the data as image data before I put it in a file ?
I don't really get how the headers applies to the data as there is no 'link' between them...

Thanks again for your help !

Re: Check "HTTP_RAW_POST_DATA" type before writing the file ?

Posted: Thu Jul 09, 2009 6:47 am
by kaisellgren
Horous wrote:I've never used regexp though, so I read some tutorials but still I'm not sure I get everything (I guess it takes a bit more time to get used to it) :
- why the '^' inside the brackets before the 'a' ? Is it to make sure there is nothing before the authorized characters ?
- why the '+' at the end ? Is this a way to make sure there is at least one character in the string ?
- if the 'i' at the end stands for 'case insensitive', what does the 'D' stand for ?
Yeah, regular expressions need some time to learn at first. To answer your questions:
- [] is basically a class. Everything inside it is matched, but if you place ^ inside [] as the first character, then it becomes a negative class. So [^a-z] matches data that is not in range of a-z.
- Yes, the + tells RegEx that there must be one or more characters (of the preceding [] class) in the data.
- Yup 'i' is case insensitive, I could type [a-zA-Z] but instead I just decided to use [a-z] and specify 'i'. I don't know what 'D' stands for, but it makes the $ character in the end to match the very of the data. So if you have "line\nanother line", without 'D', it would match "line\n", but with 'D' it matches the whole message. 'D' is used to make the $ to match the real end of the message. Otherwise it would match 1) End of data 2) New line \n 3) Carriage return \r

I highly recommend to learn to use regular expressions, this site is great: http://www.regular-expressions.info/examples.html
Horous wrote:And won't that still allow a user to create a 'file.php' ?
Even if I can now control where it is created, it would not be very hard to find that folder and execute the corrupted file...
The file would be inside your uploaded folder so it won't overwrite your application but you should note that it can overwrite other uploaded files (you can use file_exists() to find out before writing if you wish). You shouldn't upload the file anywhere in your document root; i.e., you can't upload it in a place that is directly accessible by typing in the URL. It should be uploaded above your htdocs/www/public_html/wtvr folder. If that's not possible, you could create a .htaccess and make it to deny access to those files completely, but placing files outside your document root is preferred.

site.com/index.php (/home/username/public_html/index.php)
-can't be accessed- (/home/username/uploads/some_upload.jpg)

So, if index.php wants to access the file above, it would do "../uploads/some_upload.jpg".
Horous wrote:Do I need to check the file extension as well ? Would it be efficient, as I guess it is possible to put a php script in a 'file.jpg' ?
No need. The file is served through your PHP script, so, it really does not matter what the filename and the extension is or what does it contain. If it contains PHP, Javascript or such, the web browser will display a message "could not display the image". It is important to specify Content-Type as the correct type.
Horous wrote:I don't really get how the headers applies to the data as there is no 'link' between them...
If your image is always displayed through your PHP script. It means that whenever the user views it, the web browser receives Content-Type: xxx header and will process it as a xxx.

Re: Check "HTTP_RAW_POST_DATA" type before writing the file ?

Posted: Thu Jul 09, 2009 7:09 am
by Horous
Thanks for the explanations about the regular expression, it's perfectly clear now (at least it is for this particular one).
I'll definitely take a deeper look when I have some time.

I hadn't thought of creating a folder above the public root, and now that is quite obvious...

So I check the file name with the regular expression on the basename(), I then create the file (the names are based on time and date plus a random 3 digit number, so no overwriting should occur) in a folder outside the public directory, and when I need to display it, I use the PHP script with Content-Type header.
Sounds just perfect to me... ^^

I can't try the new upload script right now for I'm at work, but with your previous answers and advices it should just be fine.

Thank you again, Kai, for your time and clear answers.
I'll post a link here when the app is online if you wish... :)