[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