Index: includes/common.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/common.inc,v retrieving revision 1.1190 diff -u -p -r1.1190 common.inc --- includes/common.inc 2 Jul 2010 15:31:10 -0000 1.1190 +++ includes/common.inc 5 Jul 2010 16:44:00 -0000 @@ -1202,10 +1202,75 @@ function flood_is_allowed($name, $thresh */ /** - * Prepare a URL for use in an HTML attribute. Strips harmful protocols. + * Strips dangerous protocols (e.g. 'javascript:') from a URI. + * + * This function must be called for all URIs within user-entered input prior + * to being output to an HTML attribute value. It is often called as part of + * check_url() or filter_xss(), but those functions return an HTML-encoded + * string, so this function can be called independently when the output needs to + * be a plain-text string for passing to t(), l(), drupal_attributes(), or + * another function that will call check_plain() separately. + * + * @param $uri + * A plain-text URI that might contain dangerous protocols. + * + * @return + * A plain-text URI stripped of dangerous protocols. As with all plain-text + * strings, this return value must not be output to an HTML page without + * check_plain() being called on it. However, it can be passed to functions + * expecting plain-text strings. + * + * @see check_url() + */ +function drupal_strip_dangerous_protocols($uri) { + static $allowed_protocols; + + if (!isset($allowed_protocols)) { + $allowed_protocols = array_flip(variable_get('filter_allowed_protocols', array('ftp', 'http', 'https', 'irc', 'mailto', 'news', 'nntp', 'rtsp', 'sftp', 'ssh', 'telnet', 'webcal'))); + } + + // Iteratively remove any invalid protocol found. + do { + $before = $uri; + $colonpos = strpos($uri, ':'); + if ($colonpos > 0) { + // We found a colon, possibly a protocol. Verify. + $protocol = substr($uri, 0, $colonpos); + // If a colon is preceded by a slash, question mark or hash, it cannot + // possibly be part of the URL scheme. This must be a relative URL, which + // inherits the (safe) protocol of the base document. + if (preg_match('![/?#]!', $protocol)) { + break; + } + // Check if this is a disallowed protocol. Per RFC2616, section 3.2.3 + // (URI Comparison) scheme comparison must be case-insensitive. + if (!isset($allowed_protocols[strtolower($protocol)])) { + $uri = substr($uri, $colonpos + 1); + } + } + } while ($before != $uri); + + return $uri; +} + +/** + * Strips dangerous protocols (e.g. 'javascript:') from a URI and encodes it for output to an HTML attribute value. + * + * @param $uri + * A plain-text URI that might contain dangerous protocols. + * + * @return + * A URI stripped of dangerous protocols and encoded for output to an HTML + * attribute value. Because it is already encoded, it should not be set as a + * value within a $attributes array passed to drupal_attributes(), because + * drupal_attributes() expects those values to be plain-text strings. To pass + * a filtered URI to drupal_attributes(), call + * drupal_strip_dangerous_protocols() instead. + * + * @see drupal_strip_dangerous_protocols() */ function check_url($uri) { - return filter_xss_bad_protocol($uri, FALSE); + return check_plain(drupal_strip_dangerous_protocols($uri)); } /** @@ -1453,45 +1518,21 @@ function _filter_xss_attributes($attr) { * @param $string * The string with the attribute value. * @param $decode - * Whether to decode entities in the $string. Set to FALSE if the $string - * is in plain text, TRUE otherwise. Defaults to TRUE. + * (Deprecated) Whether to decode entities in the $string. Set to FALSE if the + * $string is in plain text, TRUE otherwise. Defaults to TRUE. This parameter + * is deprecated and will be removed in Drupal 8. To process a plain-text URI, + * call drupal_strip_dangerous_protocols() or check_url() instead. * @return * Cleaned up and HTML-escaped version of $string. */ function filter_xss_bad_protocol($string, $decode = TRUE) { - static $allowed_protocols; - - if (!isset($allowed_protocols)) { - $allowed_protocols = array_flip(variable_get('filter_allowed_protocols', array('ftp', 'http', 'https', 'irc', 'mailto', 'news', 'nntp', 'rtsp', 'sftp', 'ssh', 'telnet', 'webcal'))); - } - // Get the plain text representation of the attribute value (i.e. its meaning). + // @todo Remove the $decode parameter in Drupal 8, and always assume an HTML + // string that needs decoding. if ($decode) { $string = decode_entities($string); } - - // Iteratively remove any invalid protocol found. - do { - $before = $string; - $colonpos = strpos($string, ':'); - if ($colonpos > 0) { - // We found a colon, possibly a protocol. Verify. - $protocol = substr($string, 0, $colonpos); - // If a colon is preceded by a slash, question mark or hash, it cannot - // possibly be part of the URL scheme. This must be a relative URL, - // which inherits the (safe) protocol of the base document. - if (preg_match('![/?#]!', $protocol)) { - break; - } - // Per RFC2616, section 3.2.3 (URI Comparison) scheme comparison must be case-insensitive - // Check if this is a disallowed protocol. - if (!isset($allowed_protocols[strtolower($protocol)])) { - $string = substr($string, $colonpos + 1); - } - } - } while ($before != $string); - - return check_plain($string); + return check_plain(drupal_strip_dangerous_protocols($string)); } /** @@ -1993,13 +2034,13 @@ function url($path = NULL, array $option ); if (!isset($options['external'])) { - // Return an external link if $path contains an allowed absolute URL. - // Only call the slow filter_xss_bad_protocol if $path contains a ':' - // before any / ? or #. - // Note: we could use url_is_external($path) here, but that would - // require another function call, and performance inside url() is critical. + // Return an external link if $path contains an allowed absolute URL. Only + // call the slow drupal_strip_dangerous_protocols() if $path contains a ':' + // before any / ? or #. Note: we could use url_is_external($path) here, but + // that would require another function call, and performance inside url() is + // critical. $colonpos = strpos($path, ':'); - $options['external'] = ($colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && filter_xss_bad_protocol($path, FALSE) == check_plain($path)); + $options['external'] = ($colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && drupal_strip_dangerous_protocols($path) == $path); } // Preserve the original path before altering or aliasing. @@ -2113,9 +2154,9 @@ function url($path = NULL, array $option */ function url_is_external($path) { $colonpos = strpos($path, ':'); - // Only call the slow filter_xss_bad_protocol if $path contains a ':' - // before any / ? or #. - return $colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && filter_xss_bad_protocol($path, FALSE) == check_plain($path); + // Only call the slow drupal_strip_dangerous_protocols() if $path contains a + // ':' before any / ? or #. + return $colonpos !== FALSE && !preg_match('![/?#]!', substr($path, 0, $colonpos)) && drupal_strip_dangerous_protocols($path) == $path; } /** Index: includes/theme.inc =================================================================== RCS file: /cvs/drupal/drupal/includes/theme.inc,v retrieving revision 1.599 diff -u -p -r1.599 theme.inc --- includes/theme.inc 7 Jun 2010 06:38:09 -0000 1.599 +++ includes/theme.inc 5 Jul 2010 16:44:01 -0000 @@ -1854,7 +1854,7 @@ function theme_item_list($variables) { * - url: The url for the link. */ function theme_more_help_link($variables) { - return '
' . t('More help', array('@link' => check_url($variables['url']))) . '
'; + return '
' . l(t('More help'), $variables['url']) . '
'; } /** @@ -1868,7 +1868,7 @@ function theme_more_help_link($variables function theme_feed_icon($variables) { $text = t('Subscribe to @feed-title', array('@feed-title' => $variables['title'])); if ($image = theme('image', array('path' => 'misc/feed.png', 'alt' => $text))) { - return '' . $image . ''; + return l($image, $variables['url'], array('html' => TRUE, 'attributes' => array('class' => array('feed-icon'), 'title' => $text))); } } @@ -1918,7 +1918,7 @@ function theme_html_tag($variables) { * - title: A descriptive verb for the link, like 'Read more'. */ function theme_more_link($variables) { - return '
' . t('More', array('@link' => check_url($variables['url']), '@title' => $variables['title'])) . '
'; + return '
' . l(t('More'), $variables['url'], array('attributes' => array('title' => $variables['title']))) . '
'; } /** @@ -2172,7 +2172,7 @@ function template_preprocess_html(&$vari if (theme_get_setting('toggle_favicon')) { $favicon = theme_get_setting('favicon'); $type = theme_get_setting('favicon_mimetype'); - drupal_add_html_head_link(array('rel' => 'shortcut icon', 'href' => check_url($favicon), 'type' => $type)); + drupal_add_html_head_link(array('rel' => 'shortcut icon', 'href' => drupal_strip_dangerous_protocols($favicon), 'type' => $type)); } // Construct page title. @@ -2355,7 +2355,7 @@ function template_preprocess_maintenance if (theme_get_setting('toggle_favicon')) { $favicon = theme_get_setting('favicon'); $type = theme_get_setting('favicon_mimetype'); - drupal_add_html_head_link(array('rel' => 'shortcut icon', 'href' => check_url($favicon), 'type' => $type)); + drupal_add_html_head_link(array('rel' => 'shortcut icon', 'href' => drupal_strip_dangerous_protocols($favicon), 'type' => $type)); } global $theme; Index: modules/comment/comment.test =================================================================== RCS file: /cvs/drupal/drupal/modules/comment/comment.test,v retrieving revision 1.83 diff -u -p -r1.83 comment.test --- modules/comment/comment.test 29 Jun 2010 00:57:19 -0000 1.83 +++ modules/comment/comment.test 5 Jul 2010 16:44:02 -0000 @@ -1190,7 +1190,7 @@ class CommentTokenReplaceTestCase extend $tests['[comment:hostname]'] = check_plain($comment->hostname); $tests['[comment:name]'] = filter_xss($comment->name); $tests['[comment:mail]'] = check_plain($this->admin_user->mail); - $tests['[comment:homepage]'] = filter_xss_bad_protocol($comment->homepage); + $tests['[comment:homepage]'] = check_url($comment->homepage); $tests['[comment:title]'] = filter_xss($comment->subject); $tests['[comment:body]'] = _text_sanitize($instance, LANGUAGE_NONE, $comment->comment_body[LANGUAGE_NONE][0], 'value'); $tests['[comment:url]'] = url('comment/' . $comment->cid, $url_options + array('fragment' => 'comment-' . $comment->cid)); Index: modules/comment/comment.tokens.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/comment/comment.tokens.inc,v retrieving revision 1.12 diff -u -p -r1.12 comment.tokens.inc --- modules/comment/comment.tokens.inc 29 Jun 2010 00:57:19 -0000 1.12 +++ modules/comment/comment.tokens.inc 5 Jul 2010 16:44:02 -0000 @@ -148,7 +148,7 @@ function comment_tokens($type, $tokens, break; case 'homepage': - $replacements[$original] = $sanitize ? filter_xss_bad_protocol($comment->homepage) : $comment->homepage; + $replacements[$original] = $sanitize ? check_url($comment->homepage) : $comment->homepage; break; case 'title': Index: modules/simpletest/tests/common.test =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/common.test,v retrieving revision 1.115 diff -u -p -r1.115 common.test --- modules/simpletest/tests/common.test 21 Jun 2010 14:59:14 -0000 1.115 +++ modules/simpletest/tests/common.test 5 Jul 2010 16:44:04 -0000 @@ -345,7 +345,7 @@ class CommonXssUnitTest extends DrupalUn public static function getInfo() { return array( 'name' => 'String filtering tests', - 'description' => 'Confirm that check_plain() and filter_xss() work correctly, including invalid multi-byte sequences.', + 'description' => 'Confirm that check_plain(), filter_xss(), and check_url() work correctly, including invalid multi-byte sequences.', 'group' => 'System', ); } @@ -372,6 +372,20 @@ class CommonXssUnitTest extends DrupalUn $text = check_plain("