Make WordPress Core

Opened 10 years ago

Last modified 9 months ago

#24157 assigned defect (bug)

safecss_filter_attr doesn't allow rgb() in inline styles

Reported by: joehoyle's profile joehoyle Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.8.1
Component: Formatting Keywords: kses has-patch has-unit-tests
Focuses: Cc:

Description

I thought there should be a ticket somewhere, but I couldn't find it!

So, safecss_filter_attr (which is used in wp_kses etc, does not allow inline styles that include rgba() etc, like his:

<span style="background: rgb(0,0,0)"></span>

I am not sure if this is intended, though not sure why it would be, there is a comment in safecss_filter_attr

if ( preg_match( '%[\\(&=}]|/\*%', $css ) ) // remove any inline css containing \ ( & } = or comments

Attachments (1)

kses.diff (1.3 KB) - added by joehoyle 10 years ago.
Unit tests for safecss_filter_attr

Download all attachments as: .zip

Change History (15)

#1 @ericlewis
10 years ago

Introduced in [11689], related: #10336

#2 @SergeyBiryukov
10 years ago

  • Version changed from 3.5.1 to 2.8.1

#3 @joehoyle
10 years ago

I wanted to open the discussion up to what css should be supported, and what is deemed insecure. AFAIK css expression as the main reason for this restriction, whereas there are a lot of CSS values that are currently not allowed due to the stripping of (, e.g. bg image urls, gradients, other css 3 "functions", rgb(a), hsla to name some.

I was going to write a patch to whitelist rgb for this ticket, however, if css expressions http://webfx.eae.net/dhtml/cssexpr/cssexpr.html are the only issue (which I believe are only available in <= IE7) then does it make more sense to blacklist "expression" rather than trying to write complex regex to whitelist the others?

@joehoyle
10 years ago

Unit tests for safecss_filter_attr

#4 @nacin
9 years ago

  • Milestone changed from Awaiting Review to Future Release

It seems to me like safecss_filter_attr() is pretty ancient and probably due for an upgrade, but that it's possible that a proper upgrade to be safe yet expansive in features would require a huge and slow library. I'm curious myself.

Thanks for the unit tests. We may be able to work around rgb(a), at the very least.

The folks who work on WordPress.com probably have the most experience with safe CSS, given their feature set. Flagging westi et al.

#5 @postpostmodern
9 years ago

I recently had safecss_filter_attr() fail when trying to embed an html 'Large Button' from iTunes linkmaker, the issue was an inline media query containing curly brackets. Which, is probably not a good practice, but it seems like it should be able to pass nonetheless. For reference, the style generated from https://linkmaker.itunes.apple.com/us/ which fails is

style="display:inline-block;overflow:hidden;background:url(https://linkmaker.itunes.apple.com/htmlResources/assets/en_us//images/web/linkmaker/badge_itunes-lrg.png) no-repeat;width:110px;height:40px;@media only screen{background-image:url(https://linkmaker.itunes.apple.com/htmlResources/assets/en_us//images/web/linkmaker/badge_itunes-lrg.svg);}"

#6 @miqrogroove
8 years ago

  • Keywords kses added

#7 @bendog
8 years ago

Still an issue in 4.2.2

I'm trying to allow some advanced comment editing for a blog's members. Most works except rgb(a) colour overrides.

#8 @paulschreiber
7 years ago

See related #37134 (with proposed filter/patch).

#9 @paulschreiber
3 years ago

Still an issue in 5.3.2.

#10 @adamsilverstein
3 years ago

The filter being added in https://core.trac.wordpress.org/ticket/37134 will enable altering the regex to avoid stripping parentheses.

#11 @swissspidy
3 years ago

  • Keywords needs-patch added
  • Milestone set to Future Release
  • Owner set to swissspidy
  • Status changed from new to assigned

I added rgb(a) support in a forked version of this function for one of my projects. I'll see if I can add a patch for it here.

This ticket was mentioned in PR #1575 on WordPress/wordpress-develop by swissspidy.


22 months ago
#12

  • Keywords has-patch has-unit-tests added; needs-patch removed
  • Added support for rgb() and rgba() values.
  • Added support for display, position, top, left, transform, opacity, white-space, clip-path, -webkit-clip-path, pointer-events and will-change properties.

Trac ticket: https://core.trac.wordpress.org/ticket/24157

#13 @markparnell
11 months ago

#56196 was marked as a duplicate.

#14 @swissspidy
9 months ago

  • Owner swissspidy deleted
Note: See TracTickets for help on using tickets.