In the code you first check the file type with finfo_file() & then when you rename the file for permanent storage you carry forward the user provided extension.
This sounds reasonable, but some recent fuzzing I've done on a prominent CMS suggests that on linux servers running php V7 file_info() returns an image mime type for files that it recognises even if the file extension is not.
This would mean that an attacker could potentially craft an arbitrary executable file which would pass mime type image filtering & then be placed where it could be run. In the case of the CMS I was testing when renaming the file I believe they use the inferred mime type to determine the extension.
Exactly! Instead of use pathinfo to get the extension, I think the best would be to have an associated array with the "mime_type" => "extension" and use the extension based on the mime type.
I believe you may have left a security hole here.
In the code you first check the file type with finfo_file() & then when you rename the file for permanent storage you carry forward the user provided extension.
This sounds reasonable, but some recent fuzzing I've done on a prominent CMS suggests that on linux servers running php V7 file_info() returns an image mime type for files that it recognises even if the file extension is not.
This would mean that an attacker could potentially craft an arbitrary executable file which would pass mime type image filtering & then be placed where it could be run. In the case of the CMS I was testing when renaming the file I believe they use the inferred mime type to determine the extension.
Exactly! Instead of use pathinfo to get the extension, I think the best would be to have an associated array with the "mime_type" => "extension" and use the extension based on the mime type.
Thank you both for your comments, I've edited the code like you said and it should be safe now :)