Skip to content

GitLab

D
drupal
Commit 9e6313e8 authored by webchick's avatar webchick
Browse files
Options

#715142 by effulgentsia, msmithgu, mr.baileys, Damien Tournoud, sun: Fixed... 

#715142 by effulgentsia, msmithgu, mr.baileys, Damien Tournoud, sun: Fixed Various URLs escaped twice, since check_url() resp. filter_xss_bad_protocol() calls check_plain().
parent 5a904b80
Hide whitespace changes
Inline Side-by-side
Showing with 105 additions and 50 deletions
includes/common.inc
View file @ 9e6313e8
......@@ -1202,10 +1202,75 @@ function flood_is_allowed($name, $threshold, $window = 3600, $identifier = NULL)
*/
/**
* 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 $options = array()) {
);
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 $options = array()) {
*/
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;
}
/**
......
includes/theme.inc
View file @ 9e6313e8
......@@ -1854,7 +1854,7 @@ function theme_item_list($variables) {
* - url: The url for the link.
*/
function theme_more_help_link($variables) {
return '<div class="more-help-link">' . t('<a href="@link">More help</a>', array('@link' => check_url($variables['url']))) . '</div>';
return '<div class="more-help-link">' . l(t('More help'), $variables['url']) . '</div>';
}
/**
......@@ -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 '<a href="' . check_url($variables['url']) . '" title="' . $text . '" class="feed-icon">' . $image . '</a>';
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 '<div class="more-link">' . t('<a href="@link" title="@title">More</a>', array('@link' => check_url($variables['url']), '@title' => $variables['title'])) . '</div>';
return '<div class="more-link">' . l(t('More'), $variables['url'], array('attributes' => array('title' => $variables['title']))) . '</div>';
}
/**
......@@ -2172,7 +2172,7 @@ function template_preprocess_html(&$variables) {
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_page(&$variables) {
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;
......
modules/comment/comment.test
View file @ 9e6313e8
......@@ -1190,7 +1190,7 @@ class CommentTokenReplaceTestCase extends CommentHelperCase {
$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));
......
modules/comment/comment.tokens.inc
View file @ 9e6313e8
......@@ -148,7 +148,7 @@ function comment_tokens($type, $tokens, array $data = array(), array $options =
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':
......
modules/simpletest/tests/common.test
View file @ 9e6313e8
......@@ -345,7 +345,7 @@ class CommonXssUnitTest extends DrupalUnitTestCase {
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 DrupalUnitTestCase {
$text = check_plain("<script>");
$this->assertEqual($text, '&lt;script&gt;', 'check_plain() escapes &lt;script&gt;');
}
/**
* Check that harmful protocols are stripped.
*/
function testBadProtocolStripping() {
// Ensure that check_url() strips out harmful protocols, and encodes for
// HTML. Ensure drupal_strip_dangerous_protocols() can be used to return a
// plain-text string stripped of harmful protocols.
$url = 'javascript:http://www.example.com/?x=1&y=2';
$expected_plain = 'http://www.example.com/?x=1&y=2';
$expected_html = 'http://www.example.com/?x=1&amp;y=2';
$this->assertIdentical(check_url($url), $expected_html, t('check_url() filters a URL and encodes it for HTML.'));
$this->assertIdentical(drupal_strip_dangerous_protocols($url), $expected_plain, t('drupal_strip_dangerous_protocols() filters a URL and returns plain text.'));
}
}
class CommonSizeTestCase extends DrupalUnitTestCase {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment