2
\$\begingroup\$

I'm trying to make a simple code to upload an image directly from a url. However I have concerns about the safety of my code since a user can submit any url. How I can make it more safe?

All the good examples I found are always related to files uploaded via a form.

So I'm having difficulties in improving this code.

$img_url = "https://i.ytimg.com/vi/wSdT-SArM2Q/maxresdefault.jpg";

function getimg($url)
{
  $headers[] = 'Accept: image/gif, image/x-bitmap, image/jpeg, image/pjpeg';
  $headers[] = 'Connection: Keep-Alive';
  $headers[] = 'Content-type: application/x-www-form-urlencoded;charset=UTF-8';
  $user_agent = 'php';
  $process = curl_init($url);
  curl_setopt($process, CURLOPT_HTTPHEADER, $headers);
  curl_setopt($process, CURLOPT_HEADER, 0);
  curl_setopt($process, CURLOPT_USERAGENT, $user_agent); //check here         
  curl_setopt($process, CURLOPT_TIMEOUT, 30);
  curl_setopt($process, CURLOPT_RETURNTRANSFER, 1);
  curl_setopt($process, CURLOPT_FOLLOWLOCATION, 1);
  $return = curl_exec($process);
  curl_close($process);
  return $return;
}

$imagename = basename($img_url);
if (file_exists('./upload/' . $imagename)) {
  //
}

$image = getimg($img_url);
file_put_contents('upload/' . $imagename, $image);
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to Code Review! Can you please edit your post to explain what might happen in the line containing the comment (i.e. //)? Would it be a block to handle the file existing already? \$\endgroup\$ Commented Feb 5, 2021 at 16:25

1 Answer 1

Reset to default
1
\$\begingroup\$

I'm trying to make a simple code to upload an image directly from a url.

You mean download.

How I can make it more safe?

Is there any reason why you want to retain the original file name, after applying the basename function ? This should protect against basic path traversal attempts like ../../ but you could as well rename the file altogether. If you are storing a record in a database, using an ID would be a good choice + adding the file extension (but not allow any file extension - see below).

This would be better than taking a chance and figuring out the possible pitfalls with your function. You are not controlling the file name and trusting user input is where the danger lies. Not to mention that there may some creative attempts involving Unicode characters or whatever. Easier said than done, but I can't guarantee your code is 100% safe at this point.

The biggest problem here is that someone could supply the URL of a webshell with a .php extension, that your script will happily download to the upload folder. If that folder is under your website root, an attacker may then be able to call http://example.com/upload/shell.php to and take over your server. It's something you can test easily.

Because that's what your script does, it will download anything upon request. At a minimum, that upload folder should not be under the website root. So the problem is not just the file name but the extension, and where that upload folder resides.

Also, there is no guarantee that the filename will be unique. You seem to be handling the scenario in some way that we don't know about.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.