UPDATE: As of WordPress 3.0, get_search_query()
sanitizes its output and can be used directly.
Theme authors: the WordPress function get_search_query()
is NOT sanitized! Don’t make the same mistake that I have in the past.
For example, this code taken from the LA-School blue theme is not safe:
<p><?php printf(__('You have searched the <a href="%1$s/">%2$s</a> blog archives for <strong>‘%3$s’</strong>.', 'laschool'), get_bloginfo('url'), get_bloginfo('name'), get_search_query()); ?></p>
With that theme, that text will only be shown if there are search results meaning the code can’t easily be exploited in this particular theme, but it’s still unsafe. The proper way would be wrap the function in attribute_escape()
like this:
<p><?php printf( __('You have searched the <a href="%1$s/">%2$s</a> blog archives for <strong>‘%3$s’</strong>.', 'laschool'), get_bloginfo('url'), get_bloginfo('name'), attribute_escape( get_search_query() ) ); ?></p>
Of course if you don’t need the search query returned and just want to out it directly, then just use the_search_query()
which will echo a totally safe version.
So when you’re saying it’s not sanitized, I’m guessing that you’re saying that it’s susceptible to SQL injection?
I just checked the codex page and it’s really sparse on any information and appears to suggest that the_search_query() and get_search_query() are synonymous. Any chance you’d be willing to edit the codex page? I was going to paste in your post, but I didn’t want to be presumptuous.
http://codex.wordpress.org/Function_Reference/get_search_query
Searched the themes in my wpmu install and none of our production themes use the function, but I did see that K2 does which I know comes with the Farms 100 pack.
Kevin on April 27th, 2009 at 6:25 AM wrote:
No, I mean if a user visits
site.com/?s=<script>alert('hi')<script>
you’ll get a Javascript alert box. It’s not escaped HTML entity wise.the_search_query() is, but it echoes rather than returning.
He’s just saying that the function returns the raw value, just as it was passed into WordPress, and you can’t trust it. Otherwise, if you just echo it back to the browser without sanitizing it yourself, you could be opening up an XSS hole (cross-site scripting). Somebody could make a link elsewhere with evil code in it which could steal an unsuspecting visitor’s cookies, under the right conditions (the user has cookies from your blog, and they get tricked into clicking the evil link on some other site).
Viper: why did you pass through attribute_escape() instead of wp_specialchars()? Wouldn’t that be a more appropriate choice for text destined to be displayed in the page? Does the extra invalid UTF check give any extra protection for this case?
Dougal Campbell on May 8th, 2009 at 5:51 AM wrote:
Probably, but they’re very similar (I believe both are just complex versions of
htmlspecialchars()
). I just stole the code I posted though out ofthe_search_query()
which is mainly meant for the search form (i.e. filling in the search box with the term you searched for).Pingback: Ma revue de la semaine | 360 e-media
Pingback: Le référencement local, les résultats personnalisés et une faille XSS | Pink Seo – Another SEO blog
the_search_query() is for outputting directly within the div body, e.g. wrapped by tags in your example.
You would echo the results of esc_attr() or esc_attr_e() for elements attributes (such as: an input value=”something” attribute). See http://codex.wordpress.org/Function_Reference/attribute_escape for deprecation notice.
At the time I wrote this post,
attribute_escape()
was not deprecated. 😉Nice article, someone just reported an XSS in my site – it was the same issue the theme was just spitting out the search query raw as %s on the page which made it easily exploited.
Switched it to the_search_query() and I’m safe again.
Thanks! Because the WP Codex information of these functions SUCKS.
The standard wp search does not sanitize user input. Where do you implement the get_search_query to get rid of the xss problem?