Merge branch 'security' into 3.1-stable
diff --git a/system/core/Loader.php b/system/core/Loader.php
index 0515723..17ff236 100644
--- a/system/core/Loader.php
+++ b/system/core/Loader.php
@@ -486,7 +486,7 @@
*/
public function view($view, $vars = array(), $return = FALSE)
{
- return $this->_ci_load(array('_ci_view' => $view, '_ci_vars' => $this->_ci_object_to_array($vars), '_ci_return' => $return));
+ return $this->_ci_load(array('_ci_view' => $view, '_ci_vars' => $this->_ci_prepare_view_vars($vars), '_ci_return' => $return));
}
// --------------------------------------------------------------------
@@ -519,19 +519,13 @@
*/
public function vars($vars, $val = '')
{
- if (is_string($vars))
- {
- $vars = array($vars => $val);
- }
+ $vars = is_string($vars)
+ ? array($vars => $val)
+ : $this->_ci_prepare_view_vars($vars);
- $vars = $this->_ci_object_to_array($vars);
-
- if (is_array($vars) && count($vars) > 0)
+ foreach ($vars as $key => $val)
{
- foreach ($vars as $key => $val)
- {
- $this->_ci_cached_vars[$key] = $val;
- }
+ $this->_ci_cached_vars[$key] = $val;
}
return $this;
@@ -940,18 +934,7 @@
* the two types and cache them so that views that are embedded within
* other views can have access to these variables.
*/
- if (is_array($_ci_vars))
- {
- foreach (array_keys($_ci_vars) as $key)
- {
- if (strncmp($key, '_ci_', 4) === 0)
- {
- unset($_ci_vars[$key]);
- }
- }
-
- $this->_ci_cached_vars = array_merge($this->_ci_cached_vars, $_ci_vars);
- }
+ empty($_ci_vars) OR $this->_ci_cached_vars = array_merge($this->_ci_cached_vars, $_ci_vars);
extract($this->_ci_cached_vars);
/*
@@ -1382,17 +1365,32 @@
// --------------------------------------------------------------------
/**
- * CI Object to Array translator
+ * Prepare variables for _ci_vars, to be later extract()-ed inside views
*
- * Takes an object as input and converts the class variables to
- * an associative array with key/value pairs.
+ * Converts objects to associative arrays and filters-out internal
+ * variable names (i.e. keys prexied with '_ci_').
*
- * @param object $object Object data to translate
+ * @param mixed $vars
* @return array
*/
- protected function _ci_object_to_array($object)
+ protected function _ci_prepare_view_vars($vars)
{
- return is_object($object) ? get_object_vars($object) : $object;
+ if ( ! is_array($vars))
+ {
+ $vars = is_object($vars)
+ ? get_object_vars($object)
+ : array();
+ }
+
+ foreach (array_keys($vars) as $key)
+ {
+ if (strncmp($key, '_ci_', 4) === 0)
+ {
+ unset($vars[$key]);
+ }
+ }
+
+ return $vars;
}
// --------------------------------------------------------------------
diff --git a/system/core/Security.php b/system/core/Security.php
index 8b313a9..585ed90 100644
--- a/system/core/Security.php
+++ b/system/core/Security.php
@@ -224,12 +224,9 @@
}
}
- // Do the tokens exist in both the _POST and _COOKIE arrays?
- if ( ! isset($_POST[$this->_csrf_token_name], $_COOKIE[$this->_csrf_cookie_name])
- OR $_POST[$this->_csrf_token_name] !== $_COOKIE[$this->_csrf_cookie_name]) // Do the tokens match?
- {
- $this->csrf_show_error();
- }
+ // Check CSRF token validity, but don't error on mismatch just yet - we'll want to regenerate
+ $valid = isset($_POST[$this->_csrf_token_name], $_COOKIE[$this->_csrf_cookie_name])
+ && hash_equals($_POST[$this->_csrf_token_name], $_COOKIE[$this->_csrf_cookie_name]);
// We kill this since we're done and we don't want to pollute the _POST array
unset($_POST[$this->_csrf_token_name]);
@@ -245,6 +242,11 @@
$this->_csrf_set_hash();
$this->csrf_set_cookie();
+ if ($valid !== TRUE)
+ {
+ $this->csrf_show_error();
+ }
+
log_message('info', 'CSRF token verified');
return $this;
}
@@ -499,7 +501,7 @@
* Becomes: <blink>
*/
$pattern = '#'
- .'<((?<slash>/*\s*)(?<tagName>[a-z0-9]+)(?=[^a-z0-9]|$)' // tag start and name, followed by a non-tag character
+ .'<((?<slash>/*\s*)((?<tagName>[a-z0-9]+)(?=[^a-z0-9]|$)|.+)' // tag start and name, followed by a non-tag character
.'[^\s\042\047a-z0-9>/=]*' // a valid attribute character immediately after the tag would count as a separator
// optional attributes
.'(?<attributes>(?:[\s\042\047/=]*' // non-attribute characters, excluding > (tag close) for obvious reasons
diff --git a/system/helpers/form_helper.php b/system/helpers/form_helper.php
index fc7d2a6..a49eea8 100644
--- a/system/helpers/form_helper.php
+++ b/system/helpers/form_helper.php
@@ -90,12 +90,6 @@
$form = '<form action="'.$action.'"'.$attributes.">\n";
- // Add CSRF field if enabled, but leave it out for GET requests and requests to external websites
- if ($CI->config->item('csrf_protection') === TRUE && strpos($action, $CI->config->base_url()) !== FALSE && ! stripos($form, 'method="get"'))
- {
- $hidden[$CI->security->get_csrf_token_name()] = $CI->security->get_csrf_hash();
- }
-
if (is_array($hidden))
{
foreach ($hidden as $name => $value)
@@ -104,6 +98,41 @@
}
}
+ // Add CSRF field if enabled, but leave it out for GET requests and requests to external websites
+ if ($CI->config->item('csrf_protection') === TRUE && strpos($action, $CI->config->base_url()) !== FALSE && ! stripos($form, 'method="get"'))
+ {
+ // Prepend/append random-length "white noise" around the CSRF
+ // token input, as a form of protection against BREACH attacks
+ if (FALSE !== ($noise = $CI->security->get_random_bytes(1)))
+ {
+ list(, $noise) = unpack('c', $noise);
+ }
+ else
+ {
+ $noise = mt_rand(-128, 127);
+ }
+
+ // Prepend if $noise has a negative value, append if positive, do nothing for zero
+ $prepend = $append = '';
+ if ($noise < 0)
+ {
+ $prepend = str_repeat(" ", abs($noise));
+ }
+ elseif ($noise > 0)
+ {
+ $append = str_repeat(" ", $noise);
+ }
+
+ $form .= sprintf(
+ '%s<input type="hidden" name="%s" value="%s" />%s%s',
+ $prepend,
+ $CI->security->get_csrf_token_name(),
+ $CI->security->get_csrf_hash(),
+ $append,
+ "\n"
+ );
+ }
+
return $form;
}
}
diff --git a/tests/codeigniter/core/Security_test.php b/tests/codeigniter/core/Security_test.php
index cbf0285..4c54ec9 100644
--- a/tests/codeigniter/core/Security_test.php
+++ b/tests/codeigniter/core/Security_test.php
@@ -154,6 +154,11 @@
'<img src="b on=">on=">"x onerror="alert(1)">',
$this->security->xss_clean('<img src="b on="<x">on=">"x onerror="alert(1)">')
);
+
+ $this->assertEquals(
+ "\n><!-\n<b d=\"'e><iframe onload=alert(1) src=x>\n<a HREF=\">\n",
+ $this->security->xss_clean("\n><!-\n<b\n<c d=\"'e><iframe onload=alert(1) src=x>\n<a HREF=\"\">\n")
+ );
}
// --------------------------------------------------------------------
diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst
index 91a59c4..ca1696c 100644
--- a/user_guide_src/source/changelog.rst
+++ b/user_guide_src/source/changelog.rst
@@ -7,6 +7,14 @@
Release Date: Not Released
+- **Security**
+
+ - Fixed an XSS vulnerability in :doc:`Security Library <libraries/security>` method ``xss_clean()``.
+ - Fixed a possible file inclusion vulnerability in :doc:`Loader Library <libraries/loader>` method ``vars()``.
+ - Fixed a possible remote code execution vulnerability in the :doc:`Email Library <libraries/email>` when 'mail' or 'sendmail' are used (thanks to Paul Buonopane from `NamePros <https://www.namepros.com/>`_).
+ - Added protection against timing side-channel attacks in :doc:`Security Library <libraries/security>` method ``csrf_verify()``.
+ - Added protection against BREACH attacks targeting the CSRF token field generated by :doc:`Form Helper <helpers/form_helper>` function :php:func:`form_open()`.
+
- General Changes
- Deprecated ``$config['allow_get_array']``.