Unverified Commit ee301cf5 authored by larowlan's avatar larowlan

SA-CORE-2018-006 by alexpott, attilatilman, bkosborne, catch, bonus, Wim...

SA-CORE-2018-006 by alexpott, attilatilman, bkosborne, catch, bonus, Wim Leers, Sam152, Berdir, Damien Tournoud, Dave Reid, Kova101, David_Rothstein, dawehner, dsnopek, samuel.mortenson, stefan.r, tedbow, xjm, timmillwood, pwolanin, njbooher, dyates, effulgentsia, klausi, mlhess, larowlan
parent 36011bea
......@@ -2311,7 +2311,10 @@ function url($path = NULL, array $options = array()) {
$language = isset($options['language']) && isset($options['language']->language) ? $options['language']->language : '';
$alias = drupal_get_path_alias($original_path, $language);
if ($alias != $original_path) {
$path = $alias;
// Strip leading slashes from internal path aliases to prevent them
// becoming external URLs without protocol. /example.com should not be
// turned into //example.com.
$path = ltrim($alias, '/');
}
}
......
......@@ -21,7 +21,7 @@ class PathTestCase extends DrupalWebTestCase {
parent::setUp('path');
// Create test user and login.
$web_user = $this->drupalCreateUser(array('create page content', 'edit own page content', 'administer url aliases', 'create url aliases'));
$web_user = $this->drupalCreateUser(array('create page content', 'edit own page content', 'administer url aliases', 'create url aliases', 'access content overview'));
$this->drupalLogin($web_user);
}
......@@ -160,6 +160,34 @@ class PathTestCase extends DrupalWebTestCase {
$this->drupalGet($edit['path[alias]']);
$this->assertNoText($node1->title, 'Alias was successfully deleted.');
$this->assertResponse(404);
// Create third test node.
$node3 = $this->drupalCreateNode();
// Create an invalid alias with a leading slash and verify that the slash
// is removed when the link is generated. This ensures that URL aliases
// cannot be used to inject external URLs.
// @todo The user interface should either display an error message or
// automatically trim these invalid aliases, rather than allowing them to
// be silently created, at which point the functional aspects of this
// test will need to be moved elsewhere and switch to using a
// programmatically-created alias instead.
$alias = $this->randomName(8);
$edit = array('path[alias]' => '/' . $alias);
$this->drupalPost('node/' . $node3->nid . '/edit', $edit, t('Save'));
$this->drupalGet('admin/content');
// This checks the link href before clicking it, rather than using
// DrupalWebTestCase::assertUrl() after clicking it, because the test
// browser does not always preserve the correct number of slashes in the
// URL when it visits internal links; using DrupalWebTestCase::assertUrl()
// would actually make the test pass unconditionally on the testbot (or
// anywhere else where Drupal is installed in a subdirectory).
$link_xpath = $this->xpath('//a[normalize-space(text())=:label]', array(':label' => $node3->title));
$link_href = (string) $link_xpath[0]['href'];
$link_prefix = base_path() . (variable_get('clean_url', 0) ? '' : '?q=');
$this->assertEqual($link_href, $link_prefix . $alias);
$this->clickLink($node3->title);
$this->assertResponse(404);
}
/**
......
......@@ -70,7 +70,9 @@ public function mail(array $message) {
// hosts. The return value of this method will still indicate whether mail
// was sent successfully.
if (!isset($_SERVER['WINDIR']) && strpos($_SERVER['SERVER_SOFTWARE'], 'Win32') === FALSE) {
if (isset($message['Return-Path']) && !ini_get('safe_mode')) {
// We validate the return path, unless it is equal to the site mail, which
// we assume to be safe.
if (isset($message['Return-Path']) && !ini_get('safe_mode') && (variable_get('site_mail', ini_get('sendmail_from')) === $message['Return-Path'] || self::_isShellSafe($message['Return-Path']))) {
// On most non-Windows systems, the "-f" option to the sendmail command
// is used to set the Return-Path. There is no space between -f and
// the value of the return path.
......@@ -109,6 +111,36 @@ public function mail(array $message) {
}
return $mail_result;
}
/**
* Disallows potentially unsafe shell characters.
*
* Functionally similar to PHPMailer::isShellSafe() which resulted from
* CVE-2016-10045. Note that escapeshellarg and escapeshellcmd are inadequate
* for this purpose.
*
* @param string $string
* The string to be validated.
*
* @return bool
* True if the string is shell-safe.
*
* @see https://github.com/PHPMailer/PHPMailer/issues/924
* @see https://github.com/PHPMailer/PHPMailer/blob/v5.2.21/class.phpmailer.php#L1430
*
* @todo Rename to ::isShellSafe() and/or discuss whether this is the correct
* location for this helper.
*/
protected static function _isShellSafe($string) {
if (escapeshellcmd($string) !== $string || !in_array(escapeshellarg($string), array("'$string'", "\"$string\""))) {
return FALSE;
}
if (preg_match('/[^a-zA-Z0-9@_\-.]/', $string) !== 0) {
return FALSE;
}
return TRUE;
}
}
/**
......
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