WordPress.org

Make WordPress Core

Opened 14 years ago

Closed 14 years ago

#4236 closed defect (bug) (fixed)

get_theme_data() doesn't clean up html in theme data.

Reported by: codein Owned by: rob1n
Milestone: 2.3 Priority: high
Severity: normal Version: 2.1.3
Component: Administration Keywords: has-patch commit
Focuses: Cc:

Description

cross site scripting is possible if someone place a line in the template/style.css file.
the value of the template-metatags should be convert to HTML entities.

example (style.css):

Version: <script>alert(document.cookie);</script>1.6

i tested it with WP-Version 2.1.3

Attachments (2)

style.css (164 bytes) - added by codein 14 years ago.
demo xss - style.css file
4236.diff (1.8 KB) - added by rob1n 14 years ago.

Download all attachments as: .zip

Change History (9)

@codein
14 years ago

demo xss - style.css file

#1 @Otto42
14 years ago

  • Priority changed from normal to high
  • Summary changed from XSS in template header of the styles.css to get_theme_data() doesn't clean up html in theme data.
  • Version set to 2.1.3

This isn't a specific XSS type of bug. The stuff pulled from the template file is not cleaned up at all, so any HTML in the theme there will show up as is on the admin pages. In theory, you could use this to steal somebody's login cookies or something if you could get them to install your theme. They wouldn't need to activate it, just to load the Presentation page.

The problem could be fixed in get_theme_data() in wp-includes/theme.php.

Suggestion: Modify get_theme_data() to run strip_tags() on everything it pulls out of the template.

Alternate suggestion: Modify get_theme_data() to run htmlentities() on that stuff instead (thus allowing greater than and less than signs in the text).

#2 @foolswisdom
14 years ago

  • Milestone changed from 2.4 to 2.2

#3 @rob1n
14 years ago

  • Keywords removed
  • Owner changed from anonymous to rob1n
  • Status changed from new to assigned

Better yet, KSES. I know for a fact many people use HTML in their Description to style it up in the admin, so it may not be a complete solution to just strip the tags or turn them into HTML entities.

Also, how "big" of an XSS risk is this, really? If you've installed a theme with this in the theme data fields, you already trust the theme owner by running the PHP code (much more dangerous, really -- passwords, etc can be sent out) on your server without any limits.

I'm +1 for fixing it, but I'm not so sure about the high priority of this.

Also, while we're at it, we could also filter it in get_plugin_data().

#4 @rob1n
14 years ago

  • Milestone changed from 2.2 to 2.3

#5 @Otto42
14 years ago

I don't see it as a particularly big deal, however it could be a way for somebody to get further into your site, if they were able to somehow add some malicious code to any installed theme's CSS file but not get into anything else.

The only "big deal" is the fact that they could make some HTML that would be active on your admin pages the moment you went to the Presentation tab, by inserting it into the name field. The theme doesn't have to be activated, the name is loaded and displayed there regardless.

@rob1n
14 years ago

#6 @rob1n
14 years ago

  • Keywords has-patch commit added; needs-patch removed

#7 @rob1n
14 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [5447]) Filter get_theme_data() data through KSES to get rid of evil XSS things. fixes #4236

Note: See TracTickets for help on using tickets.