OWASP / java-html-sanitizer

Takes third-party HTML and produces HTML that is safe to embed in your web application. Fast and easy to configure.
Other
834 stars 209 forks source link

Stripping off the contents when Child Combinator are present in the media queries #251

Closed log2akshat closed 6 months ago

log2akshat commented 2 years ago

Hi,

We are using this library in Zimbra for sanitization of the e-mail body and during sanitization of the customer-generated HTML, we came across the following situation when we have Child Combinator in the media queries and during sanitization, it is not able to parse properly and the whole HTML is stripped off.

@media yahoo {
               .d-mobile {
                  display: none !important;
               }

               .d-desktop {
                  display: block !important;
               }

               .w-lg-75>tbody>tr>td {
                  width: 75% !important;
               }

               .w-lg-100 {
                  width: 100% !important;
               }

               .w-lg-100>tbody>tr>td {
                  width: 100% !important;
               }

               .pt-lg-0>tbody>tr>td,
               .py-lg-0>tbody>tr>td {
                  padding-top: 0 !important;
               }

               .pr-lg-0>tbody>tr>td,
               .px-lg-0>tbody>tr>td {
                  padding-right: 0 !important;
               }

               .pb-lg-0>tbody>tr>td,
               .py-lg-0>tbody>tr>td {
                  padding-bottom: 0 !important;
               }

               .pl-lg-0>tbody>tr>td,
               .px-lg-0>tbody>tr>td {
                  padding-left: 0 !important;
               }
.....

It works fine if I pull the Child Combiator out of the media query I got the following message but it is able to perform sanitization without stripping off the contents.

Child:[style: null]
Next Child:null

It will be great if someone can guide me on how to handle this situation or it can be considered as an enhancement or bugfix.

log2akshat commented 2 years ago

It will be great if I can get some regarding this issue. Thanks

uttamtakalkar commented 2 years ago

OWASP Team, any updates on this issue ?

uttamtakalkar commented 2 years ago

@jmanico , can you please look into this issue

uttamtakalkar commented 2 years ago

@jmanico any updates on this ?

jmanico commented 2 years ago

I'm honestly not sure. cc @mikesamuel could you take a look at this when time allows?

log2akshat commented 2 years ago

@uttamtakalkar I think we should stop highlighting and raising a particular issue for fixing, especially personally tagging the developers of that project.

OWASP is an OpenSource community-led software project and is a nonprofit foundation doing good work voluntarily. It gets its funding through memberships, donations, gifts, and event sponsorships not like Zimbra which is run by a private company.

I guess Zimbra doesn't have any paid support from OWASP, we should be respecting the idea of OpenSource where things are done mutually by supporting each other either by contributing financially or by contributing to the project. Raising an issue is a contribution to the project but it's entirely dependent on them which issue should be prioritized and which issue can be looked at later. 

Rather than asking for updates or instructing them to look into the issue, probably it will be a better idea that Zimbra should try to fix this issue and raise a pull request for that after all it's a java project like Zimbra. It's one of the core security components Zimbra is using for a long time for sanitizing all their e-mail contents so being an OpenSource project please respect other OpenSource projects also.

subbudvk commented 7 months ago

I'm honestly not sure. cc @mikesamuel could you take a look at this when time allows?

@jmanico @mikesamuel will you be able to help on this? We are facing the same.

jmanico commented 7 months ago

I’ve not heard from Mr. Samuel in several weeks. Please give it a little more time for him to respond and if he does not, I’ll build a team to do releases for us.

mikesamuel commented 7 months ago

Sorry. I've been slammed lately. Will look at recent commits and put together a release this weekend hopefully.

jmanico commented 7 months ago

Thank you!

subbudvk commented 7 months ago

@mikesamuel Thanks Mike. When you are reviewing commits, I also request you to look at this issue as this badly strips user provided CSS, which forced us to revert to older version of library. Please review the below fix for it.