Remove custom CSS option

[justintadlock]

The WordPress.org theme review team has had a surprising number of themes lately attempting to utilize a "custom CSS" option in their themes. Some of these themes have been utilizing Redux. We've been having to block these themes from being approved for WordPress.org, primarily because of security concerns. While this is not a security bug with Redux in and of itself, it does present some sanitization issues that we can't allow if a themer is using the css type of option.

I'm opening this ticket in hopes that you will consider removing this option type from the framework for the benefit of everyone. If it's removed, theme authors won't be able to use it, which will solve part of the problem.

The framework doesn't handle sanitization properly if a theme author decides to use this option. Unless it's going to also bundle something like CSSTidy and make sure everything is actually CSS before being added to the database, theme authors really shouldn't be using that particular option type.

You can read more on this discussion in the following link (and even get involved). The comments following are of particular interest. https://make.wordpress.org/themes/2015/02/03/theme-review-chat-notes

[dovy]

@justintadlock We have a CSS sanitization callback function that developers are encouraged to use. How can we be "blamed" when developers fail to use the functions that already exist?

http://docs.reduxframework.com/core/the-basics/validation/

They need only add 'validation' => 'css' to the given field and everything is properly sanitizatized.

What more can you expect of us beyond that?

[dovy]

We use the standard WordPress wp_filter_nohtml_kses function for this sanitization. Why are we requiring devs to load a library like CSSTidy? Isn't this a little unnecessary? If it's a concern, shouldn't WordPress core have it's own built-in function instead of isolating it only to WP.com?

[dovy]

Oh and we don't have a "CSS" field. We have text areas and ace_editors that users can do with as they will. So that puts this even more beyond our control.

We'd love to comply and help. What do you suggest?

[justintadlock]

No one is blaming Redux for anything. I'm simply bringing a matter to your attention. What you do with this information is up to you. Anything y'all can do to help in this regard is nothing but a plus. As I said in my opening statement, this is not a security bug with Redux in and of itself (if it were, I would've emailed you privately). Ultimately, it's up to theme authors to make sure what they're doing is safe.

I mistakenly called this the custom CSS option type. What the real issue is that theme authors think the CSS validation done by Redux is good enough. It's not. wp_filter_nohtml_kses is not going to be allowed for this on WordPress.org.

I'm personally not the best person to ask about what code would be required to make this validation awesome. I've never built or worked on a custom CSS feature, so I wouldn't feel comfortable providing the best advice moving forward. I'd get in touch with someone on the Jetpack team (their Custom CSS module looks good) and see if they'd help with any details for such a feature in Redux.

Edit: Personally, I'd remove that validation callback and tell theme authors to roll their own for custom CSS.

On the subject of WordPress having a sanitization function for arbitrary CSS, I don't believe that'll ever happen based on what I've heard from lead devs.

[evertiro]

@dovy Given that Redux doesn't have a CSS-specific field type, and so much is left up to the developer, my suggestion would be writing a CSSTidy dropin and providing instructions on the ACE/text field docs pages on how to implement it, along with recommendations that they do so if they are implementing an inline CSS field. That should cover any perceived liability Redux has, given that the necessary information and recommendations are presented to the developer, and place any remaining liability on devs who chose not to follow that recommendation.

[dovy]

@justintadlock ok then...

I just went into JetPack and grabbed the CSSTidy settings. So now Redux CSS validation 'validate'=>'css' now fully complies with your requirements.

I think if this is a requirement of WP.org, it REALLY should be moved into the core. It's super easy and only a few lines of code.

The fix has been pushed to the most recent version of Redux: dcf323a. It will be deployed to WP.org soon.

[justintadlock]

Thanks for taking a look at things and providing a solution on your end.

I think if this is a requirement of WP.org, it REALLY should be moved into the core. It's super easy and only a few lines of code.

Handling this is currently under discussion for the theme repository on WordPress.org. TRT has little to no say into what goes into the core software. If you feel like this is something core should be handling, definitely open a Trac ticket and get the ball rolling.

[dovy]

@justintadlock I have posted a working example for any other framework to integrate the exact code I used. This is what's inside jetpack minus the pre-processors (less/scss). I am updating the README as we speak.

[dovy]

https://github.com/ReduxFramework/wp-custom-css-validation

[dovy]

@justintadlock I'd suggest you send that repo to any developers who have failed your approval because of this, or send Redux Framework users to this issue.

Redux users, this fix requires Redux 3.4.2.5+ to be used. Just be sure to add 'validate'=>'css' to any fields you are using to store custom CSS. This will not work with pre-processors however (less/saas).

[inspiraaz]

Hi dovy,

will this validation happen if i update the option outside the options panel...

$sampleReduxFramework->ReduxFramework->set('css', ccs_value');

[kprovance]

No, it won't. And folks shouldn't be doing this but in rare and extreme circumstances.

[dovy]

@justintadlock I'm rather disappointed that you approached us before you had reached any decision. As per your post here: https://make.wordpress.org/themes/2015/02/10/custom-css-boxes-in-themes/

The code we grabbed from Jetpack fails Theme-Check because CSSTidy uses file_get_contents and such. We are now reverting to the solution we had before before you involved us, namely wp_filter_nohtml_kses().

Please in the future don't post on frameworks about requirements that are not requirements at all. I spent a substantial amount of time for something that has now been rendered useless. Disappointing.

@inspiraaz We can't keep the solution as it violates themecheck. We're reverting, but the code we used can be found here. You're welcome to use it as you see fit. https://github.com/ReduxFramework/wp-custom-css-validation