Blobfolio / righteous-mimes

A comprehensive MIME and file extension tool for PHP. Finally!
Do What The F*ck You Want To Public License
2 stars 0 forks source link

PHP file accepted as PNG #9

Open chrysos opened 1 year ago

chrysos commented 1 year ago

adminer copy

VALIDATION: Naive Name: adminer copy.png Naive Extension: png Naive Type: image/png Magic Type: application/octet-stream Best Type: image/png

FINAL: Name: adminer copy.png Extension: png Type: image/png Code: 77

SYSTEM: Kernel: Darwin XXXXXXXXX 21.6.0 Darwin Kernel Version 21.6.0: Wed Oct 4 23:55:28 PDT 2023; root:xnu-8020.240.18.704.15~1/RELEASE_X86_64 x86_64 PHP: 8.1.23 Modules: Core; FFI; PDO; Phar; Reflection; SPL; SimpleXML; Zend OPcache; bcmath; bz2; calendar; cgi-fcgi; ctype; curl; date; dom; exif; fileinfo; filter; ftp; gd; gettext; hash; iconv; imagick; intl; json; libxml; mbstring; mysqli; mysqlnd; openssl; pcntl; pcre; pdo_mysql; pdo_sqlite; posix; readline; session; shmop; soap; sockets; sodium; sqlite3; standard; sysvmsg; sysvsem; sysvshm; tidy; tokenizer; xdebug; xml; xmlreader; xmlwriter; xsl; zip; zlib WordPress: 6.3.2 Plugins: xml-sitemap-generator-for-google [1.6.5] Theme: custom-theme [1.0]

chrysos commented 1 year ago

I renamed a file adminer.php to adminer copy.png on macOs.

joshstoik1 commented 1 year ago

Thanks @chrysos !

Unfortunately, I'm not sure there's a way to handle this issue without causing legitimate files to be incorrectly rejected.

application/octet-stream is basically a server's way of saying it couldn't determine the file type. It can mean just that, or that the type is wrong, or that the server is being stupid, or all of the above, as in this case. (We know adminer is really a text/x-php file!)

In short, it's a non-answer answer.

As such, both WP Core and LotF essentially just ignore application/octet-stream identifications, and fall back to the naive extension-based filtering instead. The fileinfo extension is treated like a progressive enhancement; if it is missing or can't help, validation just proceeds as best it can without it.

(Core's approach is technically stricter, but the distinction is arbitrary — you'd need to call it adminer.mp4 instead — and merely leaves valid font/, image/, and text/ files prone to false-positive rejection as a result.)

:shrug:

Do you have any thoughts?

chrysos commented 11 months ago

Hello, well I'm not an expert, but I managed to solve the specific image case by checking if it contained the image type. I understand that in other file formats this validation may not exist. But it resolved my point for now.

I hope it is useful.

if (in_array($filetype, array_keys($imageTypes))) {
    $sizes = getimagesize($filepath);
    if (!isset($sizes[2])) {
      die("File not image.");
    }
}
joshstoik1 commented 11 months ago

Thanks @chrysos !

That is definitely a good workaround for silly substitutions like this, but unfortunately won't work consistently across all the different site/server configs WP supports, especially for less common/newer image types, let alone all the non-image types that fall for the same trick.

If you have the EXIF extension installed on your server, https://www.php.net/manual/en/function.exif-imagetype.php provides a more direct third opinion to check against.