[wp-trac] [WordPress Trac] #39145: custom-background URL escaped

WordPress Trac noreply at wordpress.org
Thu Dec 8 18:34:41 UTC 2016


#39145: custom-background URL escaped
--------------------------+-----------------------------
 Reporter:  futtta        |       Owner:
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  4.7.1
Component:  Customize     |     Version:  4.7
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |     Focuses:  administration
--------------------------+-----------------------------

Comment (by westonruter):

 Well, is this not actually a problem with Autoptimize? CSS strings are
 defined as including escaped characters, also according to
 https://www.w3.org/TR/CSS21/syndata.html#uri

 I haven't seen any reports of the browsers complaining about the escaped
 slashes, so it seems the problem is more on the side of plugin.

 Also, it's not necessarily true that getting a background image URL with
 ampersands requires hacking media upload. In fact, all that is needed is a
 filter on `theme_mod_background_image` to supply a URL with ampersands or
 other “exotic” characters. I can imagine a real world use case for this
 too, for example an advertising plugin that overrides the background to
 serve up some wallpaper ad.

 One problem I do see with using `wp_json_encode()`: JS and CSS don't use
 the same mechanism for escaping unicode characters. For example, a smiley
 is encoded in CSS as `\1F601` wheres in JSON it is multi-byte
 `\ud83d\ude01`. We don't have a CSS string literal encoder available to
 us.

 The main concern I have is to prevent the background image URL from
 “breaking out” of its `url()` definition in CSS. So if the URL contains a
 `)` this would have that problem. If the string is double-quoted, then a
 `)` won't break out. Also, the reason why `json_encode()` escapes the
 forward slashes is to prevent a string like `something</script>` (or
 `something</style>` in our case) from causing the script or style tag to
 terminate and open up an injection vulnerability opportunity. However, it
 looks like `esc_url_raw()` will get us what we need here:

 * Double-quote get stripped: `esc_url_raw( 'http://example.com/evil.png
 ?"very-evil' )` => `http://example.com/evil.png?very-evil`
 * Backslashes get stripped: `esc_url_raw(
 'http://example.com/evil.png\\)very evil' )` =>
 `http://example.com/evil.png)very%20evil`

 So I think that we'd be safe doing:

 {{{#!php
 <?php
 $image = " background-image: url(" . esc_url_raw( $background ) . ");";
 }}}

 The one caveat here is that the return value of `esc_url_raw()` gets
 passed through the `clean_url` filter and so ultimately we don't know what
 the returned value will be, unfortunately.

--
Ticket URL: <https://core.trac.wordpress.org/ticket/39145#comment:7>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform


More information about the wp-trac mailing list