diff --git a/change_log.md b/change_log.md index 4c1b4c77..f03f6682 100644 --- a/change_log.md +++ b/change_log.md @@ -26,6 +26,13 @@ Most recent at top. * HTML: `Sanitizers.TABLES` allows integer `colspan` and `rowspan` on `td` and `th`; `Sanitizers.IMAGES` allows `loading="lazy|eager"`. * CSS: `text-align` accepts `start`, `end`, `justify-all` and `match-parent`. + * CSS: `calc()` is allowed in `width`, `min-width`, `max-width`, `height`, + `min-height` and `max-height` (issue #361). Operands are limited to + numbers, dimensions and percentages joined by `+`, `-`, `*`, `/` and + parentheses; anything else inside the call, including `var()`, `attr()` + and `url()`, is stripped. Custom schemas built from property names must + list `calc()` next to the sizing property to accept it, just as `color` + needs `rgb()`. * Build: Dependency version ranges replaced with pinned versions; the findbugs `jsr305`/`annotations` pair is replaced by `spotbugs-annotations`. * Build: The `empiricism` test harness is no longer published to Maven Central. @@ -43,7 +50,8 @@ Most recent at top. * Docs: README examples compile again; Javadoc links point at `latest`. * Special thanks to (in lexicographic order): Alessandro Ruzzon, corebonts, Daham Chinthana, Domi, hwangjeyeon, - Martin Jackson, strangelookingnerd, Sven Strickroth, yangbongsoo + Martin Jackson, Raibipasha-24, strangelookingnerd, Sven Strickroth, + yangbongsoo * Release 20260313.1 * Fix: Preserve the order of `rel` attribute values while still de-duplicating them. diff --git a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/CssSchema.java b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/CssSchema.java index 3a6138f1..a69cd11b 100644 --- a/owasp-java-html-sanitizer/src/main/java/org/owasp/html/CssSchema.java +++ b/owasp-java-html-sanitizer/src/main/java/org/owasp/html/CssSchema.java @@ -354,6 +354,14 @@ Property forKey(String propertyName) { Set bottomLiterals0 = j8().setOf("auto", "inherit"); Set boxShadowLiterals0 = j8().setOf( ",", "inset", "none"); + // Arithmetic inside calc(). Operands are limited to numbers, dimensions + // and percentages, so nested functions such as var(), attr() and url() + // are stripped. A bare "-" lexes as an identifier rather than as + // punctuation, but both paths consult this literal set. + Set calc$FunLiterals0 = j8().setOf( + "+", "-", "*", "/", "(", ")"); + Map calcFunctions = j8().mapOfEntries( + j8().mapEntry("calc(", "calc()")); Set clearLiterals0 = j8().setOf( "both", "inherit", "none"); Map clipFunctions = @@ -642,8 +650,11 @@ Property forKey(String propertyName) { Property fontWeight = new Property( 0, union(fontLiterals0, fontStyleLiterals0), zeroFns); builder.put("font-weight", fontWeight); - Property height = new Property(5, bottomLiterals0, zeroFns); + Property height = new Property(5, bottomLiterals0, calcFunctions); builder.put("height", height); + // top, left and right take the same values as height but are not on the + // default white-list and do not admit calc(). + Property offset = new Property(5, bottomLiterals0, zeroFns); Property letterSpacing = new Property(5, fontStyleLiterals0, zeroFns); builder.put("letter-spacing", letterSpacing); builder.put("line-height", new Property(1, fontStyleLiterals0, zeroFns)); @@ -665,7 +676,10 @@ Property forKey(String propertyName) { builder.put("list-style-type", listStyleType); Property margin = new Property(1, bottomLiterals0, zeroFns); builder.put("margin", margin); - Property maxHeight = new Property(1, maxHeightLiterals0, zeroFns); + // width, min-width and min-height take the same values as margin but + // also admit calc(). + Property width = new Property(1, bottomLiterals0, calcFunctions); + Property maxHeight = new Property(1, maxHeightLiterals0, calcFunctions); builder.put("max-height", maxHeight); Property opacity = new Property(1, mozOpacityLiterals0, zeroFns); builder.put("opacity", opacity); @@ -756,6 +770,7 @@ Property forKey(String propertyName) { builder.put("rgba()", rgb$Fun); builder.put("hsl()", rgb$Fun); builder.put("hsla()", rgb$Fun); + builder.put("calc()", new Property(5, calc$FunLiterals0, zeroFns)); @SuppressWarnings("unchecked") Property image$Fun = new Property( 18, union(mozOutlineLiterals0, rgb$FunLiterals0), mozOutlineFunctions); @@ -817,14 +832,14 @@ Property forKey(String propertyName) { builder.put("border-width", mozOutlineWidth); builder.put("cue-after", cue); builder.put("cue-before", cue); - builder.put("left", height); + builder.put("left", offset); builder.put("margin-bottom", margin); builder.put("margin-left", margin); builder.put("margin-right", margin); builder.put("margin-top", margin); builder.put("max-width", maxHeight); - builder.put("min-height", margin); - builder.put("min-width", margin); + builder.put("min-height", width); + builder.put("min-width", width); builder.put("outline", mozOutline); builder.put("outline-color", mozOutlineColor); builder.put("outline-style", mozOutlineStyle); @@ -840,13 +855,13 @@ Property forKey(String propertyName) { builder.put("pause-before", borderSpacing); builder.put("pitch-range", borderSpacing); builder.put("richness", borderSpacing); - builder.put("right", height); + builder.put("right", offset); builder.put("stress", borderSpacing); builder.put("text-indent", borderSpacing); builder.put("text-overflow", oTextOverflow); builder.put("text-shadow", boxShadow); - builder.put("top", height); - builder.put("width", margin); + builder.put("top", offset); + builder.put("width", width); builder.put("word-spacing", letterSpacing); builder.put("z-index", bottom); builder.put("repeating-linear-gradient()", linearGradient$Fun); @@ -919,6 +934,7 @@ private static Set union(Set... subsets) { "border-top-width", "border-width", "box-shadow", + "calc()", "caption-side", "color", "cue", diff --git a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/CssSchemaTest.java b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/CssSchemaTest.java index 11add4ce..b65f4253 100644 --- a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/CssSchemaTest.java +++ b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/CssSchemaTest.java @@ -27,7 +27,11 @@ package org.owasp.html; +import java.util.Arrays; import java.util.Collections; +import java.util.Map; +import java.util.Set; +import java.util.TreeSet; import org.junit.Test; @@ -81,6 +85,29 @@ public static final void testDangerousTokens() { } } + @Test + public static final void testCalcIsScopedToSizingProperties() { + Set withCalc = new TreeSet<>(); + for (Map.Entry e + : CssSchema.DEFINITIONS.entrySet()) { + if (e.getValue().fnKeys.containsKey("calc(")) { + withCalc.add(e.getKey()); + } + } + assertEquals( + new TreeSet<>(Arrays.asList( + "height", "max-height", "max-width", + "min-height", "min-width", "width")), + withCalc); + assertTrue(CssSchema.DEFAULT_WHITELIST.contains("calc()")); + CssSchema.Property calc = CssSchema.DEFAULT.forKey("calc()"); + assertNotSame(CssSchema.DISALLOWED, calc); + // Operands are quantities only: no strings, URLs, colors, words or + // nested functions. + assertEquals(CssSchema.BIT_QUANTITY | CssSchema.BIT_NEGATIVE, calc.bits); + assertTrue(calc.fnKeys.isEmpty()); + } + @Test public static final void testCustom() { CssSchema custom = CssSchema.union( diff --git a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlPolicyBuilderTest.java b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlPolicyBuilderTest.java index cb0ef937..0d1209f2 100644 --- a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlPolicyBuilderTest.java +++ b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/HtmlPolicyBuilderTest.java @@ -765,6 +765,29 @@ public final void testBackgroundImageWithUrl() { assertEquals(expected, safeHtml); } + @Test + public final void testCalcInStyleAttribute() { + PolicyFactory policy = new HtmlPolicyBuilder() + .allowStyling() + .allowElements("div") + .toFactory(); + assertEquals( + "
x
", + policy.sanitize( + "
x
")); + // Nothing executable or URL-bearing survives inside calc(). + assertEquals( + "
x
", + policy.sanitize( + "
x
")); + assertEquals( + "
x
", + policy.sanitize( + "
x
")); + } + @Test public final void testBackgroundImageWithImageFunction() { PolicyFactory policy = new HtmlPolicyBuilder() diff --git a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/StylingPolicyTest.java b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/StylingPolicyTest.java index 7ad23e97..6901314e 100644 --- a/owasp-java-html-sanitizer/src/test/java/org/owasp/html/StylingPolicyTest.java +++ b/owasp-java-html-sanitizer/src/test/java/org/owasp/html/StylingPolicyTest.java @@ -27,6 +27,7 @@ package org.owasp.html; +import java.util.Arrays; import java.util.function.Function; import javax.annotation.Nullable; @@ -218,6 +219,87 @@ public static final void testBoxProperties() { "margin:1em; margin-top:.25em"); } + @Test + public static final void testCalc() { + // https://drafts.csswg.org/css-values-4/#calc-func + // calc() is allowed in the six sizing properties (issue #361). + assertSanitizedCss( + "width:calc( 100% - 20px )", "width: calc(100% - 20px)"); + assertSanitizedCss( + "min-width:calc( 100% - 20px )", "min-width: calc(100% - 20px)"); + assertSanitizedCss( + "max-width:calc( 100% - 20px )", "max-width: calc(100% - 20px)"); + assertSanitizedCss( + "height:calc( 100% - 20px )", "height: calc(100% - 20px)"); + assertSanitizedCss( + "min-height:calc( 100% - 20px )", "min-height: calc(100% - 20px)"); + assertSanitizedCss( + "max-height:calc( 100% - 20px )", "max-height: calc(100% - 20px)"); + // All four operators, grouping, negative operands, and the function + // name is case-insensitive. + assertSanitizedCss( + "width:calc( 2 * 1em + 10px )", "width: calc(2 * 1em + 10px)"); + assertSanitizedCss( + "width:calc( ( 100% - 20px ) / 2 )", + "width: calc((100% - 20px) / 2)"); + assertSanitizedCss( + "width:calc( -1 * 20px + 100% )", "width: CALC(-1 * 20px + 100%)"); + assertSanitizedCss( + "width:calc( 100% - 20px ) !important", + "width: calc(100% - 20px) !important"); + assertSanitizedCss( + "width:calc( 100% - 20px );color:red", + "width: calc(100% - 20px); color: red"); + // Comments are dropped and unbalanced parentheses are repaired. + assertSanitizedCss( + "width:calc( 100% - 20px )", "width: calc(100%/*a*/-/*b*/20px"); + assertSanitizedCss( + "width:calc( 100% - 20px )", "width: calc(100% - 20px))"); + // Only quantities and arithmetic survive inside calc(). Anything else + // is stripped, leaving an invalid expression that browsers ignore. + assertSanitizedCss( + "width:calc( )", "width: calc(expression(alert(1337)))"); + assertSanitizedCss( + "width:calc( )", "width: calc(url('//evil.org/x'))"); + assertSanitizedCss( + "width:calc( )", "width: calc(\"//evil.org/x\")"); + assertSanitizedCss( + "width:calc( 100% - )", "width: calc(100% - var(--x))"); + assertSanitizedCss( + "width:calc( 100% - )", "width: calc(100% - attr(data-x px))"); + assertSanitizedCss( + "width:calc( 100% - )", + "width: calc(100% - env(safe-area-inset-left))"); + assertSanitizedCss( + "width:calc( 100% - )", "width: calc(100% - rgb(0, 0, 0))"); + assertSanitizedCss( + "width:calc( 100% - )", "width: calc(100% - #fff)"); + assertSanitizedCss( + "width:calc( 100% - )", "width: calc(100% - auto)"); + // calc() is not allowed in properties outside the sizing set, ... + assertSanitizedCss(null, "margin: calc(100% - 20px)"); + assertSanitizedCss(null, "padding-left: calc(100% - 20px)"); + assertSanitizedCss(null, "font-size: calc(1em + 2px)"); + assertSanitizedCss(null, "border-width: calc(1px + 1px)"); + assertSanitizedCss("margin:20px", "margin: calc(100% - 20px) 20px"); + // ... and its operators are not allowed outside calc(). + assertSanitizedCss("width:100% 20px", "width: 100% - 20px"); + assertSanitizedCss("width:20px", "width: (20px)"); + } + + @Test + public static final void testCalcRequiresOptIn() { + // As with rgb() and color, a custom schema has to list calc() next to + // the sizing property for the function to be accepted. + CssSchema widthOnly = CssSchema.withProperties(Arrays.asList("width")); + assertSanitizedCss(widthOnly, "width:20px", "width: 20px"); + assertSanitizedCss(widthOnly, null, "width: calc(100% - 20px)"); + CssSchema widthAndCalc = CssSchema.withProperties( + Arrays.asList("width", "calc()")); + assertSanitizedCss( + widthAndCalc, "width:calc( 100% - 20px )", "width: calc(100% - 20px)"); + } + @Test public static final void testLongUrls() { // Test that a long URL does not blow out the stack or consume quadratic @@ -345,8 +427,13 @@ public static final void testCdoCdc() { private static void assertSanitizedCss( @Nullable String expectedCss, String css) { + assertSanitizedCss(CssSchema.DEFAULT, expectedCss, css); + } + + private static void assertSanitizedCss( + CssSchema cssSchema, @Nullable String expectedCss, String css) { StylingPolicy stylingPolicy = new StylingPolicy( - CssSchema.DEFAULT, + cssSchema, new Function() { public String apply(String url) { String safeUrl =