7

To defend versus Remote File Inclusion where attackers try to abuse image files, I usually recommend to never use include to include image files into PHP code.

Sometimes though, the avoidance of image includes may be not possible at all (for whatever reasons, doesn't matter).

In such case I usually pickup the images somewhere in the upload process and convert them to another image format (sometimes im combination with a lossy compression) to hopefully destroy any malicious code possibly contained in the original image.

This works, but I'm not quite satisfied with it. Mainly because of the additional server load such processing produces and possible image quality decreases that sometimes may happen.

Are the any smarter ways or best practices on that?

EDIT

To clarify: I'm talking about a siutation where the attacker injects PHP code into an image file to get the injected code executed on server side after uploading the image. Forums for example allow users to upload avatars (small image files) for personalisation.

AviD
  • 73,317
  • 24
  • 140
  • 221
Jürgen Thelen
  • 181
  • 1
  • 7
  • 2
    Just curious: why would you not be able to avoid to use include? Images are HTTP resources in their own right, sothey're not "included" as such within an HTML (source) page. (You could use fopen/fpassthrough directly, for example.)

    I'd also be concerned about the harm you can do to your users: there is a certain responsibility in sending your users one of your pages with items that you don't necessarily trust, especially if they're ultimately sent by your server.

     – Bruno May 18 '11 at 20:14
  • @Bruno: I am concerned. Always. Sometimes I'm simply not allowed to touch existing code that does use include (see my comment at Jacco). Because I am concerned I want a workaround that works and eliminates the known risks in such cases. – Jürgen Thelen May 18 '11 at 21:29
  • 1
    Executing image files is a stupid practice. It's just dumb. This question is like going to a doctor and saying, "I like to ram a rusty, dirty icepick into my flesh once daily, can you recommend me a good antibiotic so I don't get infected?" What do you think the doctor is gonna say? I doubt the doctor's immediate reaction is gonna be "hey, sure, have some cipro". More likely, the answer will be closer to "quit poking yourself with an icepick!" Sometimes the only reasonable way to answer a question is to point out that the premises behind the question are ridiculous, and un-ask the question. – D.W. May 19 '11 at 06:27
  • I don't understand why you'd want to include an image file (like, a JPEG or something) and execute its contents. That seems absurd. It's not going to cause anything useful to happen. It seems pointless. If the goal is to display an image to the user of the web site, why aren't you including it in the HTML with <IMG SRC="/path/to/the.jpg">? – D.W. May 19 '11 at 06:29
  • ürgen, I think you're confused about RFI, it has nothing to do with uploading image files. Quite the opposite, its explicitly about referring to remote files, i.e. not uploaded. You seem to be referring to something more like corrupted images, or the PHP equivalent of GIFAR. Also see this question on how to validate uploaded images. – AviD May 26 '11 at 07:57

6 Answers6

7

Your PHP code should never evaluate a data file. It may print out the content of an image file, but include()ing an image file is an insane use of code. I strongly assume that what you're doing is using PHP code to serve images, and because PHP doesn't evaluate anything that isn't between "<? ?>" tags, this works in most instances. That is definitely the wrong approach.

You may be looking for the readfile() function.

Other than that, don't write include statements that take external input.

Jeff Ferland
  • 38,329
  • 9
  • 96
  • 174
  • 3
    Seriously, though, I stress again in ANY language: NEVER exec a data file. – Jeff Ferland May 18 '11 at 18:08
  • Fully agree. I myself recommend that too (see my question). But if the customer and/or his IT department does not agree to modify the code in question (because of costs, compatibility, or for whatever reasons), but wants us to build a workaround instead, then I have to deal with it. In the end the customer pays the bills, right?^^^ Anyway, I'll definitely will checkout readfile. Thanks. – Jürgen Thelen May 18 '11 at 20:42
  • 3
    I simply don't buy into the idea that they must include() images, but can't readfile() them, yet can afford to do all the lossy transformations between formats. There aren't work-arounds for this, or mitigating factors. You just don't do that. – Jeff Ferland May 18 '11 at 21:52
  • 4
    http://xkcd.com/463/ – Jeff Ferland May 19 '11 at 14:27
5

Using GD functions to convert is a nice way to solve the problem, but adds overhead as you noticed. Also why do you use include?

I guess you have done all the usual mitigations:

  • Only allow certain extensions and check against them
  • Use random file names
  • use getimagesize()
  • Check content-type

Other things you may try

  • disable execution of php files in the image upload directory
  • use a separate domain to upload the images or store them outside the webroot
john
  • 11,088
  • 2
  • 37
  • 43
  • +1 on the last two suggestions. open_beasedir-setting is worth a look. The other steps won't detect valid GIFs that contain PHP Code though. – freddyb May 18 '11 at 20:45
  • 1
    Yes, I do the usual migitations, except extension checking. I ignore file extensions completely. Using mime type detection to decide how the file is going to be handled. But heck yes, +1 for separate domain. Most of these customers already have or use a CDN. Why not moving the uploaded files over there (and out of PHP execution range at the same time)? Plain simple. Thank you, you made me slapping my face for not thinking about this myself^^ – Jürgen Thelen May 18 '11 at 20:56
3

I struggle to think a scenario where the use of include(), to include and evaluate/execute an image file, as part of another PHP-file is a valid scenario, when do you need this?
Could you elaborate on the scenario?

If you need to include() a file, it will be evaluated. And since many image formats allow comments to be added, the file could contain valid PHP-commands. So the defense would be either to copy the contents/data of the image, and store it in a new file or strip the comments.

The lossy compression is most likely an overkill (Although I guess it should be possible to forge an image that contains opcode that will be interpeted as valid PHP upon including it while at the same time being a valid image; I don't know if it would survive a lossless image copy intact).


If you are talking about uploaded images that contain PHP code that is executed by the webserver upon serving the images, the defence is much easier:

  • Do not allow file extensions other than .php to be interpeted as PHP.
    If you need dynamic PHP created .png-files, rename them .png.php and use URL rewriting to hide the .php on the client side.
  • Move the uploaded images (or any file for that matter) to a directory configured to be ignored by PHP.
  • Never store uploaded files under their original name.
Jacco
  • 7,672
  • 5
  • 33
  • 54
  • Sorry, I'm not allowed (disclosure) to elaborate further than I did in my comment at Jeff. Extensions are completely ignored, mime type detection is used (see my comment at john). Never storing uploaded files under their original names. +1 for moving files out. Thanks. – Jürgen Thelen May 18 '11 at 21:15
  • @Jürgen Thelen, I have the strong feeling you are not talking about PHP's include() at all. But rather are talking about hosting images. Also, when you would use include(), moving the files to another domain will not protect you. – Jacco May 19 '11 at 07:50
2

The Suhosin Hardened-PHP module has some protection against Remote File Include(RFI)/Local File Include(LFI) attacks. PHP's open_basedir configuration can be used to prevent LFI attacks. Such as forcing PHP to include from a directory that is read-only. RFI can be completely prevented by setting allow_url_include=Off, and this has been default for a number of years now.

Hardening your configurations is a good defense in depth approach. But its not a substitute for testing your software and patching known vulnerabilities. The w3af project is open source and can be used to test for LFI/RFI vulnerabilities. Sitewatch is what I use to make sure my web applications are safe.

rook
  • 47,238
  • 10
  • 96
  • 182
  • Yep, some of these customers have Suhosin running, but unfortunately not all, so I'd prefer a more general solution. Never heard about w3af until now, but I'll definitely have a look into it. +1 for that. Sitewatch is known to me, but never used so far. Do they disclose in detail what checks are run exactly? – Jürgen Thelen May 18 '11 at 21:10
  • @Jürgen Thelen Yeah sitewatch will test for these issues: https://sitewat.ch/Info . LFI is a form of directory traversal and I know first hand that their tests pick it up, but its not apart of the free service. – rook May 18 '11 at 21:12
0
  1. Don't use PHP, JSP, JSPX, ASP, or ASPX
  2. Don't allow any file uploads, especially images
  3. Don't use any file handling functions or classes
atdre
  • 19,072
  • 6
  • 61
  • 108
-1

Not sure about PHP, but if your image files are in an images folder, you can use a .htaccess to block all requests not coming from your own sites. (HTTP Referrer header)

Eric Falsken
  • 316
  • 2
  • 5
  • I think the question is about serving other server's content and serving them as your own, more or less acting as a reverse proxy. (HTTP Referrer can be spoofed too, so it's not ideal to protect the images you serve.) – Bruno May 18 '11 at 14:21
  • @Bruno: sorry, I should've been more clear on that. See my edit, please. – Jürgen Thelen May 18 '11 at 16:28