From 2ab1c1902711c8b0caf5c3e8f2fa825d72f6755d Mon Sep 17 00:00:00 2001
From: Andrey Andreev <narf@devilix.net>
Date: Wed, 4 Jan 2017 15:26:35 +0200
Subject: Fix an XSS vulnerability

---
 system/core/Security.php                 | 2 +-
 tests/codeigniter/core/Security_test.php | 5 +++++
 user_guide_src/source/changelog.rst      | 4 ++++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/system/core/Security.php b/system/core/Security.php
index 8b313a9a2..d198b663b 100644
--- a/system/core/Security.php
+++ b/system/core/Security.php
@@ -499,7 +499,7 @@ class CI_Security {
 		 * Becomes: &lt;blink&gt;
 		 */
 		$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/tests/codeigniter/core/Security_test.php b/tests/codeigniter/core/Security_test.php
index cbf0285ec..4c54ec9fa 100644
--- a/tests/codeigniter/core/Security_test.php
+++ b/tests/codeigniter/core/Security_test.php
@@ -154,6 +154,11 @@ class Security_test extends CI_TestCase {
 			'<img src="b on=">on=">"x onerror="alert&#40;1&#41;">',
 			$this->security->xss_clean('<img src="b on="<x">on=">"x onerror="alert(1)">')
 		);
+
+		$this->assertEquals(
+			"\n>&lt;!-\n<b d=\"'e><iframe onload=alert&#40;1&#41; 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 3ae234102..ad7d6a4ed 100644
--- a/user_guide_src/source/changelog.rst
+++ b/user_guide_src/source/changelog.rst
@@ -7,6 +7,10 @@ Version 3.1.3
 
 Release Date: Not Released
 
+-  **Security**
+
+   -  Fixed an XSS vulnerability in :doc:`Security Library <libraries/security>` method ``xss_clean()``.
+
 -  General Changes
 
    -  Deprecated ``$config['allow_get_array']``.
-- 
cgit v1.2.3-24-g4f1b


From 5a2390d4d6287f2ce35cadae4713b7dcd10fdc9b Mon Sep 17 00:00:00 2001
From: Andrey Andreev <narf@devilix.net>
Date: Wed, 4 Jan 2017 16:01:27 +0200
Subject: [ci skip] Protect CSRF verification from timing side-channel attacks

---
 system/core/Security.php            | 14 ++++++++------
 user_guide_src/source/changelog.rst |  1 +
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/system/core/Security.php b/system/core/Security.php
index d198b663b..585ed90ec 100644
--- a/system/core/Security.php
+++ b/system/core/Security.php
@@ -224,12 +224,9 @@ class CI_Security {
 			}
 		}
 
-		// 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 @@ class CI_Security {
 		$this->_csrf_set_hash();
 		$this->csrf_set_cookie();
 
+		if ($valid !== TRUE)
+		{
+			$this->csrf_show_error();
+		}
+
 		log_message('info', 'CSRF token verified');
 		return $this;
 	}
diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst
index ad7d6a4ed..7284d100c 100644
--- a/user_guide_src/source/changelog.rst
+++ b/user_guide_src/source/changelog.rst
@@ -10,6 +10,7 @@ Release Date: Not Released
 -  **Security**
 
    -  Fixed an XSS vulnerability in :doc:`Security Library <libraries/security>` method ``xss_clean()``.
+   -  Added protection against timing side-channel attacks in :doc:`Security Library <libraries/security>` method ``csrf_verify()``.
 
 -  General Changes
 
-- 
cgit v1.2.3-24-g4f1b


From cfd52edad6a4ae84b0c34755455b5b7b164878be Mon Sep 17 00:00:00 2001
From: Andrey Andreev <narf@devilix.net>
Date: Wed, 4 Jan 2017 16:58:08 +0200
Subject: [ci skip] Try to mitigate BREACH attacks against CSRF tokens

---
 system/helpers/form_helper.php      | 41 +++++++++++++++++++++++++++++++------
 user_guide_src/source/changelog.rst |  1 +
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/system/helpers/form_helper.php b/system/helpers/form_helper.php
index fc7d2a6a0..a49eea803 100644
--- a/system/helpers/form_helper.php
+++ b/system/helpers/form_helper.php
@@ -90,12 +90,6 @@ if ( ! function_exists('form_open'))
 
 		$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 @@ if ( ! function_exists('form_open'))
 			}
 		}
 
+		// 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/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst
index 7284d100c..d889d4b28 100644
--- a/user_guide_src/source/changelog.rst
+++ b/user_guide_src/source/changelog.rst
@@ -11,6 +11,7 @@ Release Date: Not Released
 
    -  Fixed an XSS vulnerability in :doc:`Security Library <libraries/security>` method ``xss_clean()``.
    -  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
 
-- 
cgit v1.2.3-24-g4f1b


From ec8dbbb79bb083acd1cf6beff5abea055b583db5 Mon Sep 17 00:00:00 2001
From: Andrey Andreev <narf@devilix.net>
Date: Wed, 4 Jan 2017 17:01:44 +0200
Subject: Fix a possible file inclusion vulnerability in CI_Loader::vars()

---
 system/core/Loader.php              | 58 ++++++++++++++++++-------------------
 user_guide_src/source/changelog.rst |  1 +
 2 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/system/core/Loader.php b/system/core/Loader.php
index 0515723b4..17ff2362c 100644
--- a/system/core/Loader.php
+++ b/system/core/Loader.php
@@ -486,7 +486,7 @@ class CI_Loader {
 	 */
 	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 @@ class CI_Loader {
 	 */
 	public function vars($vars, $val = '')
 	{
-		if (is_string($vars))
-		{
-			$vars = array($vars => $val);
-		}
-
-		$vars = $this->_ci_object_to_array($vars);
+		$vars = is_string($vars)
+			? array($vars => $val)
+			: $this->_ci_prepare_view_vars($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 @@ class CI_Loader {
 		 * 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 @@ class CI_Loader {
 	// --------------------------------------------------------------------
 
 	/**
-	 * 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/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst
index d889d4b28..fff17110e 100644
--- a/user_guide_src/source/changelog.rst
+++ b/user_guide_src/source/changelog.rst
@@ -10,6 +10,7 @@ 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()``.
    -  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()`.
 
-- 
cgit v1.2.3-24-g4f1b


From d9367b6acf96cbc407147bd4422c2cbb941ccaa1 Mon Sep 17 00:00:00 2001
From: Andrey Andreev <narf@devilix.net>
Date: Thu, 5 Jan 2017 17:35:28 +0200
Subject: Address #4963

Would supersede PR #4966
---
 system/libraries/Email.php | 54 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 7 deletions(-)

diff --git a/system/libraries/Email.php b/system/libraries/Email.php
index 4ab873586..e25015450 100644
--- a/system/libraries/Email.php
+++ b/system/libraries/Email.php
@@ -1842,6 +1842,33 @@ class CI_Email {
 
 	// --------------------------------------------------------------------
 
+	/**
+	 * Validate email for shell
+	 *
+	 * Applies stricter, shell-safe validation to email addresses.
+	 * Introduced to prevent RCE via sendmail's -f option.
+	 *
+	 * @see	https://github.com/bcit-ci/CodeIgniter/issues/4963
+	 * @see	https://gist.github.com/Zenexer/40d02da5e07f151adeaeeaa11af9ab36
+	 * @license	https://creativecommons.org/publicdomain/zero/1.0/	CC0 1.0, Public Domain
+	 *
+	 * Credits for the base concept go to Paul Buonopane <paul@namepros.com>
+	 *
+	 * @param	string	$email
+	 * @return	bool
+	 */
+	protected function _validate_email_for_shell(&$email)
+	{
+		if (function_exists('idn_to_ascii') && $atpos = strpos($email, '@'))
+		{
+			$email = self::substr($email, 0, ++$atpos).idn_to_ascii(self::substr($email, $atpos));
+		}
+
+		return (filter_var($email, FILTER_VALIDATE_EMAIL) === $email && preg_match('#\A[a-z0-9._+-]+@[a-z0-9.-]{1,253}\z#i', $email));
+	}
+
+	// --------------------------------------------------------------------
+
 	/**
 	 * Send using mail()
 	 *
@@ -1854,7 +1881,11 @@ class CI_Email {
 			$this->_recipients = implode(', ', $this->_recipients);
 		}
 
-		if ($this->_safe_mode === TRUE)
+		// _validate_email_for_shell() below accepts by reference,
+		// so this needs to be assigned to a variable
+		$from = $this->clean_email($this->_headers['Return-Path']);
+
+		if ($this->_safe_mode === TRUE || ! $this->_validate_email_for_shell($from))
 		{
 			return mail($this->_recipients, $this->_subject, $this->_finalbody, $this->_header_str);
 		}
@@ -1862,7 +1893,7 @@ class CI_Email {
 		{
 			// most documentation of sendmail using the "-f" flag lacks a space after it, however
 			// we've encountered servers that seem to require it to be in place.
-			return mail($this->_recipients, $this->_subject, $this->_finalbody, $this->_header_str, '-f '.$this->clean_email($this->_headers['Return-Path']));
+			return mail($this->_recipients, $this->_subject, $this->_finalbody, $this->_header_str, '-f '.$from);
 		}
 	}
 
@@ -1875,13 +1906,22 @@ class CI_Email {
 	 */
 	protected function _send_with_sendmail()
 	{
+		// _validate_email_for_shell() below accepts by reference,
+		// so this needs to be assigned to a variable
+		$from = $this->clean_email($this->_headers['From']);
+		if ($this->_validate_email_for_shell($from))
+		{
+			$from = '-f '.$from;
+		}
+		else
+		{
+			$from = '';
+		}
+
 		// is popen() enabled?
-		if ( ! function_usable('popen')
-			OR FALSE === ($fp = @popen(
-						$this->mailpath.' -oi -f '.escapeshellarg($this->clean_email($this->_headers['From'])).' -t'
-						, 'w'))
-		) // server probably has popen disabled, so nothing we can do to get a verbose error.
+		if ( ! function_usable('popen')	OR FALSE === ($fp = @popen($this->mailpath.' -oi '.$from.' -t', 'w')))
 		{
+			// server probably has popen disabled, so nothing we can do to get a verbose error.
 			return FALSE;
 		}
 
-- 
cgit v1.2.3-24-g4f1b


From 61fd92498db72bc511effa8c15274596afbb5010 Mon Sep 17 00:00:00 2001
From: Andrey Andreev <narf@devilix.net>
Date: Fri, 6 Jan 2017 11:47:34 +0200
Subject: [ci skip] Add a changelog entry for #4963

---
 user_guide_src/source/changelog.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst
index fff17110e..b60a90949 100644
--- a/user_guide_src/source/changelog.rst
+++ b/user_guide_src/source/changelog.rst
@@ -11,6 +11,7 @@ Release Date: Not Released
 
    -  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()`.
 
-- 
cgit v1.2.3-24-g4f1b