diff options
-rw-r--r-- | system/core/Loader.php | 58 | ||||
-rw-r--r-- | system/core/Security.php | 16 | ||||
-rw-r--r-- | system/database/DB_forge.php | 2 | ||||
-rw-r--r-- | system/database/DB_query_builder.php | 2 | ||||
-rw-r--r-- | system/helpers/form_helper.php | 41 | ||||
-rw-r--r-- | system/libraries/Email.php | 58 | ||||
-rw-r--r-- | system/libraries/Image_lib.php | 39 | ||||
-rw-r--r-- | tests/codeigniter/core/Loader_test.php | 6 | ||||
-rw-r--r-- | tests/codeigniter/core/Security_test.php | 5 | ||||
-rw-r--r-- | user_guide_src/source/changelog.rst | 30 | ||||
-rw-r--r-- | user_guide_src/source/installation/downloads.rst | 3 | ||||
-rw-r--r-- | user_guide_src/source/installation/upgrade_314.rst | 14 | ||||
-rw-r--r-- | user_guide_src/source/installation/upgrading.rst | 3 | ||||
-rw-r--r-- | user_guide_src/source/libraries/form_validation.rst | 2 |
14 files changed, 207 insertions, 72 deletions
diff --git a/system/core/Loader.php b/system/core/Loader.php index 987679550..e9813a7c9 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); /** @@ -1371,17 +1354,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 prefixed 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($vars) + : 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 0c187e72f..1c398632d 100644 --- a/system/core/Security.php +++ b/system/core/Security.php @@ -225,12 +225,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]); @@ -246,6 +243,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; } @@ -500,7 +502,7 @@ class CI_Security { * 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/database/DB_forge.php b/system/database/DB_forge.php index 83b646d6e..b52d1fb2b 100644 --- a/system/database/DB_forge.php +++ b/system/database/DB_forge.php @@ -348,7 +348,7 @@ abstract class CI_DB_forge { if (($result = $this->db->query($sql)) !== FALSE) { - empty($this->db->data_cache['table_names']) OR $this->db->data_cache['table_names'][] = $table; + isset($this->db->data_cache['table_names']) && $this->db->data_cache['table_names'][] = $table; // Most databases don't support creating indexes from within the CREATE TABLE statement if ( ! empty($this->keys)) diff --git a/system/database/DB_query_builder.php b/system/database/DB_query_builder.php index 3f1a8021a..ab19d97a2 100644 --- a/system/database/DB_query_builder.php +++ b/system/database/DB_query_builder.php @@ -1553,7 +1553,7 @@ abstract class CI_DB_query_builder extends CI_DB_driver { is_bool($escape) OR $escape = $this->_protect_identifiers; - $keys = array_keys($this->_object_to_array(current($key))); + $keys = array_keys($this->_object_to_array(reset($key))); sort($keys); foreach ($key as $row) diff --git a/system/helpers/form_helper.php b/system/helpers/form_helper.php index 9844c752a..4a4a7c89f 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/system/libraries/Email.php b/system/libraries/Email.php index bfff8953b..1483f2203 100644 --- a/system/libraries/Email.php +++ b/system/libraries/Email.php @@ -450,7 +450,6 @@ class CI_Email { $this->_headers = array(); $this->_debug_msg = array(); - $this->set_header('User-Agent', $this->useragent); $this->set_header('Date', $this->_set_date()); if ($clear_attachments !== FALSE) @@ -1824,6 +1823,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() * * @return bool @@ -1835,9 +1861,18 @@ class CI_Email { $this->_recipients = implode(', ', $this->_recipients); } + // _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->_validate_email_for_shell($from)) + { + return mail($this->_recipients, $this->_subject, $this->_finalbody, $this->_header_str); + } + // 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); } // -------------------------------------------------------------------- @@ -1849,13 +1884,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; } diff --git a/system/libraries/Image_lib.php b/system/libraries/Image_lib.php index ebcfc6748..8b5a1adb0 100644 --- a/system/libraries/Image_lib.php +++ b/system/libraries/Image_lib.php @@ -392,6 +392,16 @@ class CI_Image_lib { $this->initialize($props); } + /** + * A work-around for some improperly formatted, but + * usable JPEGs; known to be produced by Samsung + * smartphones' front-facing cameras. + * + * @see https://github.com/bcit-ci/CodeIgniter/issues/4967 + * @see https://bugs.php.net/bug.php?id=72404 + */ + ini_set('gd.jpeg_ignore_warning', 1); + log_message('info', 'Image Lib Class Initialized'); } @@ -1644,25 +1654,26 @@ class CI_Image_lib { $this->set_error('imglib_invalid_image'); return FALSE; } + $types = array(1 => 'gif', 2 => 'jpeg', 3 => 'png'); - $mime = (isset($types[$vals[2]])) ? 'image/'.$types[$vals[2]] : 'image/jpg'; + $mime = isset($types[$vals[2]]) ? 'image/'.$types[$vals[2]] : 'image/jpg'; if ($return === TRUE) { return array( - 'width' => $vals[0], - 'height' => $vals[1], - 'image_type' => $vals[2], - 'size_str' => $vals[3], - 'mime_type' => $mime - ); - } - - $this->orig_width = $vals[0]; - $this->orig_height = $vals[1]; - $this->image_type = $vals[2]; - $this->size_str = $vals[3]; - $this->mime_type = $mime; + 'width' => $vals[0], + 'height' => $vals[1], + 'image_type' => $vals[2], + 'size_str' => $vals[3], + 'mime_type' => $mime + ); + } + + $this->orig_width = $vals[0]; + $this->orig_height = $vals[1]; + $this->image_type = $vals[2]; + $this->size_str = $vals[3]; + $this->mime_type = $mime; return TRUE; } diff --git a/tests/codeigniter/core/Loader_test.php b/tests/codeigniter/core/Loader_test.php index c1c4997c4..241c415b3 100644 --- a/tests/codeigniter/core/Loader_test.php +++ b/tests/codeigniter/core/Loader_test.php @@ -295,8 +295,10 @@ class Loader_test extends CI_TestCase { $output->expects($this->once())->method('append_output')->with($content.$value); $this->ci_instance_var('output', $output); - // Test view output - $this->assertInstanceOf('CI_Loader', $this->load->view($view, array($var => $value))); + // Test view output and $vars as an object + $vars = new stdClass(); + $vars->$var = $value; + $this->assertInstanceOf('CI_Loader', $this->load->view($view, $vars)); } // -------------------------------------------------------------------- diff --git a/tests/codeigniter/core/Security_test.php b/tests/codeigniter/core/Security_test.php index 2e1127f87..4dd31f4b1 100644 --- a/tests/codeigniter/core/Security_test.php +++ b/tests/codeigniter/core/Security_test.php @@ -155,6 +155,11 @@ class Security_test extends CI_TestCase { '<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 ed814aa22..7e52a0eda 100644 --- a/user_guide_src/source/changelog.rst +++ b/user_guide_src/source/changelog.rst @@ -99,13 +99,35 @@ Release Date: Not Released - Removed the second (out of three) parameter from the :php:func:`form_upload()` function (it was never used). -Version 3.1.3 +Version 3.1.4 ============= Release Date: Not Released - General Changes + - Updated the :doc:`Image Manipulation Library <libraries/image_lib>` to work-around an issue with some JPEGs when using GD. + +Bug fixes for 3.1.4 +------------------- + +- Fixed a regression (#4975) - :doc:`Loader Library <libraries/loader>` couldn't handle objects passed as view variables. + +Version 3.1.3 +============= + +Release Date: Jan 09, 2017 + +- **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']``. - Deprecated ``$config['standardize_newlines']``. - Deprecated :doc:`Date Helper <helpers/date_helper>` function :php:func:`nice_date()`. @@ -128,6 +150,12 @@ Bug fixes for 3.1.3 - Fixed a bug (#4937) - :doc:`Image Manipulation Library <libraries/image_lib>` method ``initialize()`` didn't translate *new_image* inputs to absolute paths. - Fixed a bug (#4941) - :doc:`Query Builder <database/query_builder>` method ``order_by()`` didn't work with 'RANDOM' under the 'pdo/sqlite' driver. - Fixed a regression (#4892) - :doc:`Query Builder <database/query_builder>` method ``update_batch()`` didn't properly handle identifier escaping. +- Fixed a bug (#4953) - :doc:`Database Forge <database/forge>` method ``create_table()`` didn't update an internal tables list cache if it exists but is empty. +- Fixed a bug (#4958) - :doc:`Query Builder <database/query_builder>` method ``count_all_results()`` didn't take into account cached ``ORDER BY`` clauses. +- Fixed a bug (#4804) - :doc:`Query Builder <database/query_builder>` method ``insert_batch()`` could fail if the input array pointer was modified. +- Fixed a bug (#4962) - :doc:`Database Force <database/forge>` method ``alter_table()`` would fail with the 'oci8' driver. +- Fixed a bug (#4457) - :doc:`Image Manipulation Library <libraries/image_lib>` method ``get_image_properties()`` didn't detect invalid images. +- Fixed a bug (#4765) - :doc:`Email Library <libraries/email>` didn't send the ``User-Agent`` header without a prior call to ``clear()``. Version 3.1.2 ============= diff --git a/user_guide_src/source/installation/downloads.rst b/user_guide_src/source/installation/downloads.rst index ae58e796f..d36296e35 100644 --- a/user_guide_src/source/installation/downloads.rst +++ b/user_guide_src/source/installation/downloads.rst @@ -3,7 +3,8 @@ Downloading CodeIgniter ####################### - `CodeIgniter v3.2.0-dev (Current version) <https://codeload.github.com/bcit-ci/CodeIgniter/zip/develop>`_ -- `CodeIgniter v3.1.3-dev <https://codeload.github.com/bcit-ci/CodeIgniter/zip/3.1-stable>`_ +- `CodeIgniter v3.1.4-dev <https://codeload.github.com/bcit-ci/CodeIgniter/zip/3.1-stable>`_ +- `CodeIgniter v3.1.3 <https://codeload.github.com/bcit-ci/CodeIgniter/zip/3.1.3>`_ - `CodeIgniter v3.1.2 <https://codeload.github.com/bcit-ci/CodeIgniter/zip/3.1.2>`_ - `CodeIgniter v3.1.1 <https://codeload.github.com/bcit-ci/CodeIgniter/zip/3.1.1>`_ - `CodeIgniter v3.1.0 <https://codeload.github.com/bcit-ci/CodeIgniter/zip/3.1.0>`_ diff --git a/user_guide_src/source/installation/upgrade_314.rst b/user_guide_src/source/installation/upgrade_314.rst new file mode 100644 index 000000000..3f2da6564 --- /dev/null +++ b/user_guide_src/source/installation/upgrade_314.rst @@ -0,0 +1,14 @@ +############################# +Upgrading from 3.1.3 to 3.1.4 +############################# + +Before performing an update you should take your site offline by +replacing the index.php file with a static one. + +Step 1: Update your CodeIgniter files +===================================== + +Replace all files and directories in your *system/* directory. + +.. note:: If you have any custom developed files in these directories, + please make copies of them first. diff --git a/user_guide_src/source/installation/upgrading.rst b/user_guide_src/source/installation/upgrading.rst index 14127d42e..ca7677ba9 100644 --- a/user_guide_src/source/installation/upgrading.rst +++ b/user_guide_src/source/installation/upgrading.rst @@ -8,7 +8,8 @@ upgrading from. .. toctree:: :titlesonly: - Upgrading from 3.1.2+ to 3.2.x <upgrade_320> + Upgrading from 3.1.3+ to 3.2.x <upgrade_320> + Upgrading from 3.1.3 to 3.1.4 <upgrade_314> Upgrading from 3.1.2 to 3.1.3 <upgrade_313> Upgrading from 3.1.1 to 3.1.2 <upgrade_312> Upgrading from 3.1.0 to 3.1.1 <upgrade_311> diff --git a/user_guide_src/source/libraries/form_validation.rst b/user_guide_src/source/libraries/form_validation.rst index 65fd9acc8..6a92cc983 100644 --- a/user_guide_src/source/libraries/form_validation.rst +++ b/user_guide_src/source/libraries/form_validation.rst @@ -985,7 +985,7 @@ Rule Parameter Description **valid_url** No Returns FALSE if the form element does not contain a valid URL. **valid_email** No Returns FALSE if the form element does not contain a valid email address. **valid_emails** No Returns FALSE if any value provided in a comma separated list is not a valid email. -**valid_ip** No Returns FALSE if the supplied IP is not valid. +**valid_ip** Yes Returns FALSE if the supplied IP address is not valid. Accepts an optional parameter of 'ipv4' or 'ipv6' to specify an IP format. **valid_mac** No Returns FALSE if the supplied MAC address is not valid. **valid_base64** No Returns FALSE if the supplied string contains anything other than valid Base64 characters. |