esc_url() doesn't allow protocol-relative URLs with colons

[SergeyBiryukov]

This doesn't work:

wp_enqueue_style( 'twentytwelve-fonts', "//fonts.googleapis.com/css?family=Open+Sans:400italic,700italic,400,700", array(), null );

The colon is the culprit. wp_kses_bad_protocol() reduces the URL to:

400italic,700italic,400,700

So esc_url() returns an empty string:
​http://core.trac.wordpress.org/browser/tags/3.4.2/wp-includes/formatting.php#L2572

[SergeyBiryukov]

21974.patch​ only calls wp_kses_bad_protocol() if the URL doesn't start with a slash. There's a similar detection earlier:
​http://core.trac.wordpress.org/browser/tags/3.4.2/wp-includes/formatting.php#L2559

Perhaps wp_kses_bad_protocol() should be patched instead?

[nacin]

Technically, a colon is a "reserved" character which means outside of its official use, it must be encoded. This reminds me of #16859.

[SergeyBiryukov]

Related: #23922

[SergeyBiryukov]

#23853 was marked as a duplicate.

[SergeyBiryukov]

Replying to nacin:

Technically, a colon is a "reserved" character which means outside of its official use, it must be encoded.

An encoded colon (: or :) doesn't work either.

wp_kses_bad_protocol_once() splits by any of those values:
​http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/kses.php#L1053

And esc_url() still returns an empty string.

I guess we shouldn't call wp_kses_bad_protocol() at all for a protocol-relative URL. Refreshed the patch.

21974.2.patch​ just fixes the issue.

21974.3.patch​ also skips the strtolower() check, which is redundant in this case.

[chengas123]

#23853 was marked as a duplicate.

[CoenJacobs]

Replying to nacin:

Technically, a colon is a "reserved" character which means outside of its official use, it must be encoded. This reminds me of #16859.

It does the same with URLs containing the colon as a split between URL and port.

[markjaquith]

Weird quirk: the bug does not trigger when the path is "/"

Triggers the issue: //example.com/foo?foo=abc:def

Does not: //example.com/?foo=abc:def

See: [1296/tests]

[markjaquith]

This needs a security review.

[nacin]

21974.3.patch​ looks good to me. Asking duck_ and mdawaffe to examine closer.

[nacin]

Per duck_, this is good. A few other security libraries don't do *anything* when the first character is /, #, or ? — which reminded us both of #22951, the esc_url() performance ticket.

[nacin]

In 24642:

Skip protocol checking in esc_url() when we are dealing with a relative URL. Prevents munging of colons in paths and query strings, when present in a protocol-relative URL.

props SergeyBiryukov.
fixes #21974.

[mdawaffe]

Replying to markjaquith:

Weird quirk: the bug does not trigger when the path is "/"

Yeah - there's a strange exception is kses for this case.

[miqrogroove]

Replying to SergeyBiryukov:

Replying to nacin:

Technically, a colon is a "reserved" character which means outside of its official use, it must be encoded.

An encoded colon (: or :) doesn't work either.

For future reference, the correct encoding is %3a or %3A which is not equivalent to the colon HTML character entity.