ezyang / htmlpurifier

Standards compliant HTML filter written in PHP
http://htmlpurifier.org
GNU Lesser General Public License v2.1
3.07k stars 327 forks source link

Allow secure inline webp images #295

Closed gsantner closed 4 months ago

gsantner commented 3 years ago

Hello,

I use Pico on NextCloud, which uses htmlpurifier to clean the contents.

However, even with inline images enabled, it will filter out any webp inline images.
Can you please update the relevant code to also allow inline webp images (unlike svg, it's just image data too, so don't see an issue here).

Example: <img alt='title.webp' src='data:image/webp;base64,UklGRpBdAABXRUJQVlA..............' />
Embedding small icons as webp allows to have just one document, without 10 dependency assets files :D.

For reference, where this bug was discovered https://github.com/picocms/Pico/issues/588

Thank you!

gsantner commented 3 years ago

enough to update this array?

grafik

ezyang commented 3 years ago

yes, assuming ImageMagick understands webp

gsantner commented 3 years ago

I use IM to convert images to webp ... so yes 😃

PHPGangsta commented 3 years ago

It depends on the version of ImageMagick of cause. It looks like "6.8.9-9" has a working implementation, but sometimes you additionally need to install "webp" or "cwebp" or "libwebp-dev" to make it work? See:

https://askubuntu.com/questions/251950/imagemagick-convert-cant-convert-to-webp/659262#659262

Maybe you should check if the installed version of ImageMagick understands the format before adding it to the allowed_types array? See https://www.php.net/manual/de/imagick.queryformats.php

Also: The functions getimagesize() and image_type_to_mime_type() are used inside HTMLPurifier_URIScheme_data::doValidate(). The constants IMG_WEBP and IMAGETYPE_WEBP are defined since PHP 7.0.10, but htmlpurifier has a minimum PHP version of 5.2 according to the composer.json. https://www.php.net/manual/en/image.constants.php#constant.img-webp

You have to be careful, just adding it to the $allowed_types array is not enough... There is a comment saying this explicitly (see screenshot above):

// you better write validation code for other types if you
// decide to allow them
gsantner commented 2 years ago

Can be closed as anyway won't be fixed? A bit sad, there is more than png & jpg in 2021 :/.

ezyang commented 2 years ago

I'm happy to accept a patch that does this. I'm guessing that it isn't even that hard, just need to add a test.

gsantner commented 4 months ago

I don't use this library or any software using it anymore, so closing. After ~3 years don't expect it to get fixed. Also I won't develop on it (as I don't use it).

If somebody comes here, suggestions: Mark avif, webp, jxl inline as allowed. It's not the jpg + png stoneage anymore 😄

pyres01 commented 4 months ago

邮件已收到,谢谢!