PHP JPG image re-creation from original file

Question

A third party can upload files via FTP to a client's webserver. These images should be displayed on a public-available webpage. The upload directory is not accessible via normal HTTP. To increase security I thought it might be a good idea to re-create the images and store it in the web-folder manually.

The import format is always JPG. The function will return the new filename if the image could be created or null otherwise.

Is there any way to improve it? Especially all the "early returns" look kind of ugly.

function processImage($filename, $importPath, $storagePath, $quality = 80) {
 $source = $importPath . $filename . '.jpg';
 $target = $filename . '_processed.jpg';
 $storage = $storagePath . $target;
 if (!file_exists($source)) {
 return null;
 }
 $image = imagecreatefromstring(file_get_contents($source));
 if (!$image) {
 return null;
 }
 $dimension = [
 imagesx($image),
 imagesy($image)
 ];
 $newImage = imagecreatetruecolor($dimension[0], $dimension[1]);
 if (!$newImage) {
 return null;
 }
 if (!imagecopyresampled($newImage, $image, 0, 0, 0, 0, $dimension[0], $dimension[1], $dimension[0], $dimension[1])) {
 return null;
 }
 if (!imagejpeg($newImage, $storage, $quality)) {
 return null;
 }
 return $target;
}

PS: The filename is available without the extension .jpg as a string. In case you worry about the first line.

• TimSparrow: I do not see the 'early returns' ugly - if you make long if-else brancges, then it would be ugly.
• Conor Mancone: @Iwrestledabearonce. you can always search or post on security.stackexchange, but with my own familiarity with JS/JPEG polygots and security in general, exploiting such a vulnerability is difficult, and what the OP is doing most likely will remove any malicious code. You can probably get some good opinions on security, but like with all stack exchange sites, make sure you read the rules-of-conduct before posting there.
• Conor Mancone: @Iwrestledabearonce. apologies if you felt I was being argumentative. I wasn't trying to avoid explaining or being argumentative: rather, this is a lot for a comment thread which is why I suggested opening a question on security. The reason why this would strip out malicious code doesn't have to do with meta data. The 'resampling' in imagecopyresampled will redraw the image: the resampling will effectively randomize the bits in the image, destroying any ascii data that is hiding in it.
• Conor Mancone: Also, the attack itself only works in limited circumstances: primarily (but maybe not exclusively), the image has to be loaded in a script tag, which is unusual, or takes additional effort to make happen. This limits the effectiveness of such an attack. Hosting a JPG file with malicious data embedded doesn't immediately pose a risk to anyone, except perhaps by giving an attacker a way to circumvent CSP protections if they find an XSS vulnerability. Again, security.stackexchange.com is a great place to discuss such a thing in detail.
• I wrestled a bear once.: @ConorMancone - thank you for clearing that up for me. no need to post on security, you were very concise with your explanation. +1

Answer 1

You may use this advice from stackexchange:

$im = imagecreatefrompng(input_filename)
imagejpeg($im, output_filename);

A call to imagecopyresample in your example is quite resource-consuming and can cause image quality loss. The call above is the same as 'open-save' in any graphical editor. I doubt that it leaves any malicious data intact, as imagejpeg re-compresses the raw image again.

Another option is to use exif_imagetype:

exif_imagetype() reads the first bytes of an image and checks its signature.

This prevents upload of non-graphical files masked as graphical, but does not eliminate a file that contains a jpeg signature, but is not jpeg.