Do not run kses on display filters for front page views

[ryan]

Several display filters run wp_kses_data and other heavyweight functions. These functions are already run when saving. They were added to the display filters as a defense-in-depth for the possibility of an exploit sneaking things into the DB. Running these on the display causes a serious performance hit, however. wp_list_bookmarks() running kses on the link fields can burn up 10% of the total page load time. Let's limit running these functions to admin page displays. Displaying bad fields in the admin is more dangerous since those fields can cover their tracks. We can lose the belt and suspenders approach for front page displays where performance is more critical.

[nacin]

It's also worth noting that we're often already escaping these values, and that if the person has access to exploit the database, you're hosed anyway.

[ryan]

(In [15559]) Do not run kses on display filters for front page views. see #14758

[demetris]

Ryan, how do you measure the performance of filters?

(Just curious to know and learn. When I try new things I check with the WP-Profile plugin -- ​http://devel.kostdoktorn.se/wp-profile-plugin -- by Johan Eenfeldt.)

[Denis-de-Bernardy]

Isn't this contrary to security best practices? I mean, the database is obviously not supposed to contain insecure data. But it remains an untrusted source: if an SQL injection prone plugin allows anything malicious into it, this ticket ensures we're removing our last line of defense against XSS vulnerabilities and so forth.

[nacin]

I believe we are still properly escaping in attributes and such.

[Denis-de-Bernardy]

Replying to nacin:

I believe we are still properly escaping in attributes and such.

Try manually updating, into your database, a category and set its description to:

XSS<script>alert('Evil script that exploits an IE security hole');</script>

And then visit that category on your site's front page. Assuming it gets displayed on your theme, I would assume [15559] allows the alert to show.

[demetris]

Replying to Denis-de-Bernardy:

Replying to nacin:

I believe we are still properly escaping in attributes and such.

[SNIP] I would assume [15559] allows the alert to show.

It does.

[nacin]

A description doesn't fall into an attribute, so it wouldn't be escaped.

Try doing the same thing in post content.

[ryan]

Performance measured with xdebug and cachegrind. When Rasmus does his PHP performance presentation, we get dinged for how slow this is, particularly in wp_list_bookmarks() -> sanitize_bookmark_field() -> apply_filters() -> wp_kses_data(). Imagine if we did kses on post_content and post_title during display.

[ryan]

Hmmm, ​http://talks.php.net/presentations/slides/intro/wp_xdebug.png

[demetris]

Thanks for the answer, Ryan.

I did not know about that presentation by Rasmus. I watched it and I found it interesting that the only part where he found significant room for improvement was wp_list_bookmarks() (which many people don’t use anyway).

[jane]

@ryan: status check on this one?