Make WordPress Core

Opened 10 hours ago

Last modified 2 hours ago

#65838 new defect (bug)

Formatting: CSS function name matching in safecss_filter_attr() is case-sensitive

Reported by: wildworks Owned by:
Priority: normal Milestone: Awaiting Review
Component: Formatting Version:
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses:

Description

Split off from a review comment on PR #12942.

CSS function names are ASCII case-insensitive, so CALC(1px + 2px), rotatex(45deg), and LINEAR-GRADIENT(red, blue) are all valid CSS. However, every function-matching regular expression in safecss_filter_attr() is case-sensitive, so valid declarations using anything other than all-lowercase function names are silently dropped.

// "width: calc(100% - 10px)"
var_dump( safecss_filter_attr( 'width: calc(100% - 10px)' ) );

// "" — expected to be preserved
var_dump( safecss_filter_attr( 'width: CALC(100% - 10px)' ) );

Make the function matching case-insensitive, covering the url(), gradient, and generic function patterns alike. The change is small but relaxes the sanitizer rather than tightening it, so it needs strict checking that no malformed or unsafe value slips through,

Change History (2)

This ticket was mentioned in PR #12949 on WordPress/wordpress-develop by @anupkankale.


2 hours ago
#1

  • Keywords has-patch has-unit-tests added

CSS function names are ASCII case-insensitive, so CALC(1px + 2px), URL(foo.jpg), and LINEAR-GRADIENT(red, blue) are all valid CSS. Every function-matching pattern in safecss_filter_attr() was case-sensitive, however, so valid declarations using anything other than all-lowercase function names were silently dropped.

Add the i modifier to the url(), gradient, and generic function patterns, and lowercase the value before the url( / -gradient( sniffing done for custom properties.

Only the function names are matched loosely; the grammar each pattern accepts is unchanged, so a mixed-case value is now held to exactly the same standard as its lowercase equivalent. Protocol checking in wp_kses_bad_protocol() was already case-insensitive, so URL() values are validated the same way url() values are.

Property names remain case-sensitive, which is a separate concern.

Fixes #65838. See #64974.

Trac ticket:

## Use of AI Tools

#2 @anupkankale
2 hours ago

PR: https://github.com/WordPress/wordpress-develop/pull/12949

Adds the i modifier to the function-matching patterns in safecss_filter_attr():

  • /url\([^)]+\)/i — the url(*) sequence match.
  • /^url\(\s*(['"]?)(.*)(\g1)\s*\)$/i — the strict url() cleanup that extracts the URL for protocol checking.
  • /(?:repeating-)?(?:linear|radial|conic)-gradient\((?:[^()]|\([^()]*\))*\)/i — the gradient match, which also picks up mixed-case nested functions such as RGBA().
  • /\b(?:var|calc|min|max|minmax|clamp|repeat)(\((?:[^()]|(?1))*\))/i — the generic CSS function match.

Custom property values are also lowercased before the url( / -gradient( sniffing, since that path detects the value type by string comparison rather than by property name.

On the "needs strict checking that nothing unsafe slips through" point in the description:

  • Only the function names are matched loosely. Each pattern's accepted grammar is unchanged, so a mixed-case value is held to exactly the same standard as its lowercase equivalent — malformed and unbalanced values are still dropped.
  • wp_kses_bad_protocol_once2() already lowercases the scheme before comparing it against the allowed protocols, so URL(JavaScript:...) is rejected exactly the way url(javascript:...) is.
  • Previously a mixed-case function left its ( in the test string and the declaration was dropped by the [\\\(&=}]|/\* check. Every value newly allowed here now goes through the same URL/gradient/function validation its lowercase form always has.

Unit tests cover both directions. Newly preserved: CALC(), Calc(Var()), CLAMP(MIN(), ..., MAX()), REPEAT(4, MINMAX()), URL("foo.jpg"), LINEAR-GRADIENT() with nested RGBA(), Repeating-Radial-Gradient(), and custom properties holding URL() / CONIC-GRADIENT() values. Still dropped: URL("bad://..."), uRl( "JavaScript:alert(1)" ), empty URL(), LINEAR-GRADIENT(red, URL(javascript:alert(1))), unbalanced CALC(3em + 10px, EXPRESSION(...), UNKNOWN-GRADIENT(...), and CALCMAX(...).

Note that property names remain case-sensitive — Text-transform: capitalize is still dropped, and there is an existing test asserting that. That felt like a separate question from function-name matching, so I left it alone here. Happy to open a follow-up ticket if it's worth changing.

Note: See TracTickets for help on using tickets.