1

After reading this blog post I realized that getimagesize() doesn't provide absolute safety so I decided to use imagepng based on this answer. However to be able to use imagepng from image that is uploaded via xmr request, I need to use first this:

$input = fopen("php://input","r");
$temp = tmpfile();
$target = fopen($path,"w")
fseek($tamp,0,SEEK_SET)
stream_copy_to_stream($temp,$target_file_name)

Then I can use

$sourceImg = @imagecreatefromstring(@file_get_contents($source));
if ($sourceImg === false) {
  throw new Exception("{$source}: Invalid image.");
}
$width = imagesx($sourceImg);
$height = imagesy($sourceImg);
$targetImg = imagecreatetruecolor($width, $height);
imagecopy($targetImg, $sourceImg, 0, 0, 0, 0, $width, $height);
imagedestroy($sourceImg);
imagepng($targetImg, $target);
imagedestroy($targetImg);

If image contains some malicious code, could in this case using fopen and stream_copy_to_stream posses any risk? If so, is there any better way if image is uploaded with xmr?

EDIT: As @your-common-sense pointed I could simply use imagecreatefromstring(file_get_contents("php://input"));. However now the question is if using imagecreatefromstring(file_get_contents("php://input")); posses any risk.

JohnyFree
  • 1,319
  • 3
  • 22
  • 35
  • 4
    I don't get the point of all this stream paraphernalia. Can't you JUST do imagecreatefromstring(file_get_contents("php://input"));? – Your Common Sense Jun 04 '23 at 08:15
  • Can you unify these variable names? – shingo Jun 04 '23 at 10:07
  • 2
    The risk starts, when you save the data to a file.. it _could_ be some malicious content, which now exists physically on your server. This is the typical loophole for code injection attacks. – Honk der Hase Jun 04 '23 at 10:28
  • @YourCommonSense great point. Is there still any risk using imagecreatefromstring(file_get_contents("php://input"));? – JohnyFree Jun 04 '23 at 10:56
  • 1
    The risk here would be if the gd library contained some flaw which could be triggered with some specifically crafted content and would lead to some exploitable buffer overflow or something of that sort. You can peruse the known bug reports and vulnerability lists to see if anything comes up that you might need to worry about. – deceze Jun 04 '23 at 11:29
  • @deceze yep you are right, php version up to 7.2.1. had this vulnerability https://www.cvedetails.com/cve/CVE-2018-5711/ so it is not safe to use with that php version. I couldn't find any vulnerability for later versions. – JohnyFree Jun 04 '23 at 11:40

1 Answers1

0

imagecreatefromstring() is a reasonably safe way to sanitize an uploaded image file as long as your php and web server code are kept up to the most recent security patch level. Sanitize === check it for malicious or malformed contents.

Be sure to generate the image file extension (.jpg, .png, etc) from the actual contents of the image file before you store the image on your server filesystem for later download. Don't trust the file extension passed to you by the person uploading the image. Something like this will work.

$size = getimagesizefromstring( whatever );
$extension = image_type_to_extension( $size[2] );

Sanitize the filename passed to you as well. It should be, for best cross-platform results, all lower case. And it should contain no DIRECTORY_SEPARATOR, /, or . characters.

O. Jones
  • 103,626
  • 17
  • 118
  • 172