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
850 stars 214 forks source link

xxx-large font-size is discarded when allowStyling() is used #291

Closed Ashish-Singh-B closed 9 months ago

Ashish-Singh-B commented 11 months ago

I am sanitazing the input using the default CssSchema and discovered that xxx-large font size is not supported in allowed font-sizes in styling

ImmutableSet<String> fontLiterals1 = ImmutableSet.of(
        "large", "larger", "small", "smaller", "x-large", "x-small",
        "xx-large", "xx-small");

I tried various ways like 1) union of custom CssSchema with just xxx-large font-size along with default CssSchema but it results as

Caused by: java.lang.IllegalArgumentException: Duplicate irreconcilable definitions for font-size
    at org.owasp.html.CssSchema.union(CssSchema.java:199)

2) I also tried creating customHTMLPolicyBuilder with allowAttributes settings but it seems to be applied after the font-size is removed by CssSchema


CustomerHtmlPolicyBuilder.allowAttributes("style").matching(Pattern.compile("font-size:xxx-large"))
            .onElements("span")

please let me know how to proceed with font-size as xxx-large as it's a valid size and supported in various browsers

Example :


String to sanitize: 
the <span style="font-size:xxx-large">large</span> formatting issue with chrome

Expected: (same as above as all valid configurations)
the <span style="font-size:xxx-large">large</span> formatting issue with chrome

Actual: 
the large formatting issue with chrome

the large formatting issue with chrome
Ashish-Singh-B commented 11 months ago

can we add xxx-large font in list of allowed font sizes

ImmutableSet<String> fontLiterals1 = ImmutableSet.of(
        "large", "larger", "small", "smaller", "x-large", "x-small",
        "xx-large", "xx-small");
ThaKarakostas commented 11 months ago

try to create your own CssSchema with CssSchema.withProperties and then use htmlPolicyBuilder.allowStyling(CssSchema.union(customCssSchema))

Ashish-Singh-B commented 11 months ago

I tried creating my custom CssSchema with xxx-large font-size and tried union operation with default CssSchema but since the default Schema already has policy defined for font-size so the code explicitly throws duplicate definition error

 throw new IllegalArgumentException(
              "Duplicate irreconcilable definitions for " + name);
subbudvk commented 10 months ago

The description says, xxx-large is having issue while the example html string it is xx-large . I written a test to verify this behaviour and it looks to be passing to me.

Output : the <span style="font-size:xxx-large">large</span> formatting issue with chrome

Can you please let me know if I am testing it wrong?

 @Test
  public static final void testCSSFontSize() {
     HtmlPolicyBuilder builder = new HtmlPolicyBuilder();
     PolicyFactory factory = builder.allowElements("span")
     .allowAttributes("style").onElements("span").toFactory();
     String toSanitize = "the <span style=\"font-size:xxx-large\">large</span> formatting issue with chrome";
     assertEquals(toSanitize, factory.sanitize(toSanitize)); 
  }
Ashish-Singh-B commented 10 months ago

The description says, xxx-large is having issue while the example html string it is xx-large . I written a test to verify this behaviour and it looks to be passing to me.

Output : the <span style="font-size:xxx-large">large</span> formatting issue with chrome

Can you please let me know if I am testing it wrong?

 @Test
  public static final void testCSSFontSize() {
   HtmlPolicyBuilder builder = new HtmlPolicyBuilder();
   PolicyFactory factory = builder.allowElements("span")
   .allowAttributes("style").onElements("span").toFactory();
   String toSanitize = "the <span style=\"font-size:xxx-large\">large</span> formatting issue with chrome";
   assertEquals(toSanitize, factory.sanitize(toSanitize)); 
  }

updated the sample input to xxx-large in original comment, also you are testing it wrong, as soon as you append .allowStyling() in your htmlbuilder your span tag with xxx-large would be discarded

Ashish-Singh-B commented 10 months ago

Thanks a lot @subbudvk for fixing the issue, any idea on how to proceed next, how to get it merged and released