[wp-trac] [WordPress Trac] #19414: Filter 'kses_allowed_protocols' is only applied once in function wp_allowed_protocols() & function esc_url() returns empty string;

[WordPress Trac]

#19414: Filter 'kses_allowed_protocols' is only applied once in function
wp_allowed_protocols()  & function esc_url() returns empty string;
--------------------------+-------------------------------
 Reporter:  Anatta        |      Owner:
     Type:  defect (bug)  |     Status:  new
 Priority:  normal        |  Milestone:  Awaiting Review
Component:  Security      |    Version:  3.3
 Severity:  major         |   Keywords:  reporter-feedback
--------------------------+-------------------------------
 '''Files:''' wp-includes/functions.php: ''Lines:4665-4674'' and wp-
 includes/formatting.php ''Lines: 2339 to 2342''.

 ''apply_filters( 'kses_allowed_protocols', $protocols );'' is only called
 when ''wp_allowed_protocols()'' is first initiated.  This makes it
 impossible to grant a temporary protocol exception.

 I discovered this in a plugin trying to add a javascript link to the admin
 bar.  Function ''esc_url()'' (wp-includes/formatting.php) would normally
 return blank for ''javascript:myFunction(/* code */);'', however, I want
 to temporarily allow the javascript protocol between the
 ''wp_before_admin_bar_render'' and ''wp_after_admin_bar_render actions''
 by applying a filter.

 This is currently impossible because the other possible filter,
 ''clean_url'' is not fired in ''esc_url'' if ''wp_kses_bad_protocol()''
 returns false.  The only current option to allow javascript as a protocol
 on the admin bar is to add a permanent filter - not great for security
 should it become known that plugins with working javascript on the admin
 bar (e.g. a Facebook Connect logout function) must have the javascript
 protocol allowed site-wide.

 ''The current wp_allowed_protocols() in functions.php:''
 {{{
 function wp_allowed_protocols() {
         static $protocols;

         if ( empty( $protocols ) ) {
                 $protocols = array( 'http', 'https', 'ftp', 'ftps',
 'mailto', 'news', 'irc', 'gopher', 'nntp', 'feed', 'telnet', 'mms',
 'rtsp', 'svn' );
                 $protocols = apply_filters( 'kses_allowed_protocols',
 $protocols );
         }

         return $protocols;
 }
 }}}

 '''Fix suggestion 1''': Move the apply_filter outside the ''if ( empty(
 $protocols ) )'' statement:

 ''Patched wp_allowed_protocols()''
 {{{
 function wp_allowed_protocols() {
         static $protocols;

         if ( empty( $protocols ) ) {
                 $protocols = array( 'http', 'https', 'ftp', 'ftps',
 'mailto', 'news', 'irc', 'gopher', 'nntp', 'feed', 'telnet', 'mms',
 'rtsp', 'svn' );
         }

         $protocols = apply_filters( 'kses_allowed_protocols', $protocols
 );

         return $protocols;
 }
 }}}

 '''Fix suggestion 2''': Change ''esc_url()'' in wp-includes/formatting.php
 not to ''return'' directly, but to set ''$url'' to empty instead of
 directly returning an empty string, allowing ''$url'' to pass through the
 ''clean_url'' filter.

 ''Patched esc_url()'' (Changed line 2342 from ''return'' to ''$url ='')
 {{{
 function esc_url( $url, $protocols = null, $_context = 'display' ) {
         $original_url = $url;

         if ( '' == $url )
                 return $url;
         $url =
 preg_replace('|[^a-z0-9-~+_.?#=!&;,/:%@$\|*\'()\\x80-\\xff]|i', '', $url);
         $strip = array('%0d', '%0a', '%0D', '%0A');
         $url = _deep_replace($strip, $url);
         $url = str_replace(';//', '://', $url);
         /* If the URL doesn't appear to contain a scheme, we
          * presume it needs http:// appended (unless a relative
          * link starting with /, # or ? or a php file).
          */
         if ( strpos($url, ':') === false && ! in_array( $url[0], array(
 '/', '#', '?' ) ) &&
                 ! preg_match('/^[a-z0-9-]+?\.php/i', $url) )
                 $url = 'http://' . $url;

         // Replace ampersands and single quotes only when displaying.
         if ( 'display' == $_context ) {
                 $url = wp_kses_normalize_entities( $url );
                 $url = str_replace( '&', '&', $url );
                 $url = str_replace( "'", ''', $url );
         }

         if ( ! is_array( $protocols ) )
                 $protocols = wp_allowed_protocols();
         if ( wp_kses_bad_protocol( $url, $protocols ) != $url )
                 $url = '';

         return apply_filters('clean_url', $url, $original_url, $_context);
 }
 }}}

 One of both of these minor patches before 3.3 final would be greatly
 appreciated.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/19414>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software