Xss:filter() malforms inline image references with data uri scheme

Always wary of changing this code but this looks like a sensible and self-contained change, however I added a question to the MR about applying the new condition.

Out of scope for this, but I wonder if there is a way of reusing the Masterminds HTML5 parser in the XSS filter instead of having separate code that attempts to parse HTML attributes.

Added lots more comments that I thought of, mostly around the regex, as these can be brittle and there might be some sneaky ways around it.

@longwave Replacing this code with something more modern would surely be welcome. When I ported KSES to become filter_xss WhatWG was but a year old and HTML5 didn't exist for another two years yet. And as far as I can see, this code didn't change at all in the near two decades since.

Follow Mozilla's guidelines and support inline images with “data:image/*” unless it’s “data:image/svg+xml”.

I recommend we go the other way and use a whitelist of known safe mimetypes. Who knows what the future brings. Or if I intentionally mangle the format to be something like "data:image/_svg" can I get the web browser to still render this as an svg?

#8 seems like a good idea over having a regex that contains exceptions.

Thanks for all the valuable feedback, everyone!

Or if I intentionally mangle the format to be something like "data:image/_svg" can I get the web browser to still render this as an svg?

Well, should not this be the concern of browsers rather than us? I used the Mozilla blog post as a source because I found no other, but if anybody knows what the current logic is that Firefox/Chrome/Safari or others use to decide safe data URI-s for images, please share.

@longwave Replacing this code with something more modern would surely be welcome. When I ported KSES to become filter_xss for Drupal 4.5.6/4.6.4 WhatWG was but a year old and HTML5 didn't exist for another two years yet. And as far as I can see, this code didn't change at all in the near two decades since.

Yeah.. as part of my investigation, I also concluded that this code is so old... and it made me wonder if and when we should adopt symfony/html-sanitizer, was that already considered? (Again, Drupal core issue search yielded no result.)

I recommend we go the other way and use a whitelist of known safe mimetypes.

Does anybody can come up with a sane default list? I generally like and prefer the whitelisting-based solution, but I wonder (again) if browsers are doing the same nowadays; based on this older Mozilla article, they are not.

I recommend we go the other way and use a whitelist of known safe mimetypes.

Does anybody can come up with a sane default list? I generally like and prefer the whitelisting-based solution, but I wonder (again) if browsers are doing the same nowadays; based on this older Mozilla article, they are not.

Maybe this MDN doc could provide us with a sane default list?
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#supported_...

If this is the direction we should take, should we allow overriding this list with a new parameter in \Drupal\Component\Utility\Xss::filter()?

Opened ✨ Adopt Symfony's HTML Sanitizer Active as a potential follow up.

We're still using D10.2.x in a project and need to roll the MR for 10.2.x, I'll give it a go!

Before patching:

Broken image in rich_text filtered content when all attributes are allowed for the img element.

After patching and rebuilding caches:

Image is displayed as expected.

Thank you for this fix, it is an important one!