Fix byte-safety issues & actually test for them
diff --git a/.gitignore b/.gitignore
index 5982f9b..eb45390 100644
--- a/.gitignore
+++ b/.gitignore
@@ -8,6 +8,8 @@
!application/logs/index.html
!application/logs/.htaccess
+composer.lock
+
user_guide_src/build/*
user_guide_src/cilexer/build/*
user_guide_src/cilexer/dist/*
diff --git a/.travis.yml b/.travis.yml
index 6b90a5f..2a0be4d 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -26,11 +26,10 @@
- sh -c "if [ '$DB' = 'pgsql' ] || [ '$DB' = 'pdo/pgsql' ]; then psql -c 'create database ci_test;' -U postgres; fi"
- sh -c "if [ '$DB' = 'mysql' ] || [ '$DB' = 'mysqli' ] || [ '$DB' = 'pdo/mysql' ]; then mysql -e 'create database IF NOT EXISTS ci_test;'; fi"
-script: phpunit -d zend.enable_gc=0 -d date.timezone=UTC --coverage-text --configuration tests/travis/$DB.phpunit.xml
+script: phpunit -d zend.enable_gc=0 -d date.timezone=UTF -d mbstring.func_overload=7 -d mbstring.internal_encoding=UTF-8 vendor/bin/phpunit --coverage-text --configuration tests/travis/$DB.phpunit.xml
matrix:
allow_failures:
- - php: 5.3
- php: hhvm
exclude:
- php: hhvm
diff --git a/composer.json b/composer.json
index 64d1be1..0a898d2 100644
--- a/composer.json
+++ b/composer.json
@@ -17,6 +17,7 @@
"paragonie/random_compat": "Provides better randomness in PHP 5.x"
},
"require-dev": {
- "mikey179/vfsStream": "1.1.*"
+ "mikey179/vfsStream": "1.1.*",
+ "phpunit/phpunit": "4.* || 5.*"
}
-}
\ No newline at end of file
+}
diff --git a/system/helpers/text_helper.php b/system/helpers/text_helper.php
index 07c01c3..217729b 100644
--- a/system/helpers/text_helper.php
+++ b/system/helpers/text_helper.php
@@ -138,7 +138,10 @@
function ascii_to_entities($str)
{
$out = '';
- for ($i = 0, $s = strlen($str) - 1, $count = 1, $temp = array(); $i <= $s; $i++)
+ $length = defined('MB_OVERLOAD_STRING')
+ ? mb_strlen($str, '8bit') - 1
+ : strlen($str) - 1;
+ for ($i = 0, $count = 1, $temp = array(); $i <= $length; $i++)
{
$ordinal = ord($str[$i]);
@@ -176,7 +179,7 @@
$temp = array();
}
// If this is the last iteration, just output whatever we have
- elseif ($i === $s)
+ elseif ($i === $length)
{
$out .= '&#'.implode(';', $temp).';';
}
diff --git a/system/libraries/Encrypt.php b/system/libraries/Encrypt.php
index 46f3747..ebcc6e8 100644
--- a/system/libraries/Encrypt.php
+++ b/system/libraries/Encrypt.php
@@ -122,7 +122,7 @@
$key = config_item('encryption_key');
- if ( ! strlen($key))
+ if ( ! self::strlen($key))
{
show_error('In order to use the encryption class requires that you set an encryption key in your config file.');
}
@@ -252,7 +252,7 @@
$string = $this->_xor_merge($string, $key);
$dec = '';
- for ($i = 0, $l = strlen($string); $i < $l; $i++)
+ for ($i = 0, $l = self::strlen($string); $i < $l; $i++)
{
$dec .= ($string[$i++] ^ $string[$i]);
}
@@ -275,7 +275,8 @@
{
$hash = $this->hash($key);
$str = '';
- for ($i = 0, $ls = strlen($string), $lh = strlen($hash); $i < $ls; $i++)
+
+ for ($i = 0, $ls = self::strlen($string), $lh = self::strlen($hash); $i < $ls; $i++)
{
$str .= $string[$i] ^ $hash[($i % $lh)];
}
@@ -295,7 +296,7 @@
public function mcrypt_encode($data, $key)
{
$init_size = mcrypt_get_iv_size($this->_get_cipher(), $this->_get_mode());
- $init_vect = mcrypt_create_iv($init_size, MCRYPT_RAND);
+ $init_vect = mcrypt_create_iv($init_size, MCRYPT_DEV_URANDOM);
return $this->_add_cipher_noise($init_vect.mcrypt_encrypt($this->_get_cipher(), $key, $data, $this->_get_mode(), $init_vect), $key);
}
@@ -313,13 +314,14 @@
$data = $this->_remove_cipher_noise($data, $key);
$init_size = mcrypt_get_iv_size($this->_get_cipher(), $this->_get_mode());
- if ($init_size > strlen($data))
+ if ($init_size > self::strlen($data))
{
return FALSE;
}
- $init_vect = substr($data, 0, $init_size);
- $data = substr($data, $init_size);
+ $init_vect = self::substr($data, 0, $init_size);
+ $data = self::substr($data, $init_size);
+
return rtrim(mcrypt_decrypt($this->_get_cipher(), $key, $data, $this->_get_mode(), $init_vect), "\0");
}
@@ -339,7 +341,7 @@
$key = $this->hash($key);
$str = '';
- for ($i = 0, $j = 0, $ld = strlen($data), $lk = strlen($key); $i < $ld; ++$i, ++$j)
+ for ($i = 0, $j = 0, $ld = self::strlen($data), $lk = self::strlen($key); $i < $ld; ++$i, ++$j)
{
if ($j >= $lk)
{
@@ -369,7 +371,7 @@
$key = $this->hash($key);
$str = '';
- for ($i = 0, $j = 0, $ld = strlen($data), $lk = strlen($key); $i < $ld; ++$i, ++$j)
+ for ($i = 0, $j = 0, $ld = self::strlen($data), $lk = self::strlen($key); $i < $ld; ++$i, ++$j)
{
if ($j >= $lk)
{
@@ -477,4 +479,43 @@
return hash($this->_hash_type, $str);
}
+ // --------------------------------------------------------------------
+
+ /**
+ * Byte-safe strlen()
+ *
+ * @param string $str
+ * @return int
+ */
+ protected static function strlen($str)
+ {
+ return defined('MB_OVERLOAD_STRING')
+ ? mb_strlen($str, '8bit')
+ : strlen($str);
+ }
+
+ // --------------------------------------------------------------------
+
+ /**
+ * Byte-safe substr()
+ *
+ * @param string $str
+ * @param int $start
+ * @param int $length
+ * @return string
+ */
+ protected static function substr($str, $start, $length = NULL)
+ {
+ if (defined('MB_OVERLOAD_STRING'))
+ {
+ // mb_substr($str, $start, null, '8bit') returns an empty
+ // string on PHP 5.3
+ isset($length) OR $length = ($start >= 0 ? self::strlen($str) - $start : -$start);
+ return mb_substr($str, $start, $length, '8bit');
+ }
+
+ return isset($length)
+ ? substr($str, $start, $length)
+ : substr($str, $start);
+ }
}
diff --git a/system/libraries/Encryption.php b/system/libraries/Encryption.php
index 74832ed..c1e454d 100644
--- a/system/libraries/Encryption.php
+++ b/system/libraries/Encryption.php
@@ -135,11 +135,11 @@
);
/**
- * mbstring.func_override flag
+ * mbstring.func_overload flag
*
* @var bool
*/
- protected static $func_override;
+ protected static $func_overload;
// --------------------------------------------------------------------
@@ -161,7 +161,7 @@
show_error('Encryption: Unable to find an available encryption driver.');
}
- isset(self::$func_override) OR self::$func_override = (extension_loaded('mbstring') && ini_get('mbstring.func_override'));
+ isset(self::$func_overload) OR self::$func_overload = (extension_loaded('mbstring') && ini_get('mbstring.func_overload'));
$this->initialize($params);
if ( ! isset($this->_key) && self::strlen($key = config_item('encryption_key')) > 0)
@@ -911,7 +911,7 @@
*/
protected static function strlen($str)
{
- return (self::$func_override)
+ return (self::$func_overload)
? mb_strlen($str, '8bit')
: strlen($str);
}
@@ -928,7 +928,7 @@
*/
protected static function substr($str, $start, $length = NULL)
{
- if (self::$func_override)
+ if (self::$func_overload)
{
// mb_substr($str, $start, null, '8bit') returns an empty
// string on PHP 5.3
diff --git a/tests/codeigniter/libraries/Encryption_test.php b/tests/codeigniter/libraries/Encryption_test.php
index 96e52ad..99c5d4b 100644
--- a/tests/codeigniter/libraries/Encryption_test.php
+++ b/tests/codeigniter/libraries/Encryption_test.php
@@ -94,10 +94,22 @@
}
// Test default length, it must match the digest size
- $this->assertEquals(64, strlen($this->encryption->hkdf('foobar', 'sha512')));
+ $hkdf_result = $this->encryption->hkdf('foobar', 'sha512');
+ $this->assertEquals(
+ 64,
+ defined('MB_OVERLOAD_STRING')
+ ? mb_strlen($hkdf_result, '8bit')
+ : strlen($hkdf_result)
+ );
// Test maximum length (RFC5869 says that it must be up to 255 times the digest size)
- $this->assertEquals(12240, strlen($this->encryption->hkdf('foobar', 'sha384', NULL, 48 * 255)));
+ $hkdf_result = $this->encryption->hkdf('foobar', 'sha384', NULL, 48 * 255);
+ $this->assertEquals(
+ 12240,
+ defined('MB_OVERLOAD_STRING')
+ ? mb_strlen($hkdf_result, '8bit')
+ : strlen($hkdf_result)
+ );
$this->assertFalse($this->encryption->hkdf('foobar', 'sha224', NULL, 28 * 255 + 1));
// CI-specific test for an invalid digest
diff --git a/tests/phpunit.xml b/tests/phpunit.xml
index 96c3af9..875198c 100644
--- a/tests/phpunit.xml
+++ b/tests/phpunit.xml
@@ -17,10 +17,8 @@
</testsuite>
</testsuites>
<filter>
- <blacklist>
- <directory suffix=".php">PEAR_INSTALL_DIR</directory>
- <directory suffix=".php">PHP_LIBDIR</directory>
- <directory suffix=".php">../vendor</directory>
- </blacklist>
+ <whitelist>
+ <directory suffix=".php">../system/</directory>
+ </whitelist>
</filter>
-</phpunit>
\ No newline at end of file
+</phpunit>
diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst
index 2769990..17069ca 100644
--- a/user_guide_src/source/changelog.rst
+++ b/user_guide_src/source/changelog.rst
@@ -7,6 +7,12 @@
Release Date: Not Released
+- **Security**
+
+ - Updated :doc:`Encrypt Library <libraries/encrypt>` (DEPRECATED) to call ``mcrypt_create_iv()`` with ``MCRYPT_DEV_URANDOM``.
+ - Fixed byte-safety issues in :doc:`Encrypt Library <libraries/encrypt>` (DEPRECATED) when ``mbstring.func_overload`` is enabled.
+ - Fixed byte-safety issues in :doc:`Encryption Library <libraries/encryption>` when ``mbstring.func_overload`` is enabled.
+
- General Changes
- Updated the :doc:`Image Manipulation Library <libraries/image_lib>` to work-around an issue with some JPEGs when using GD.
@@ -18,6 +24,7 @@
- Fixed a bug (#4977) - :doc:`Loader Library <libraries/loader>` method ``helper()`` could accept any character as a filename extension separator.
- Fixed a regression where the :doc:`Session Library <libraries/sessions>` would fail on a ``session_regenerate_id(TRUE)`` call with the 'database' driver.
- Fixed a bug (#4987) - :doc:`Query Builder <database/query_builder>` caching didn't keep track of table aliases.
+- Fixed a bug where :doc:`Text Helper <helpers/text_helper>` function ``ascii_to_entities()`` wasn't byte-safe when ``mbstring.func_overload`` is enabled.
Version 3.1.3
=============
@@ -82,7 +89,7 @@
- Fixed a regression (#4874) - :doc:`Session Library <libraries/sessions>` didn't take into account ``session.hash_bits_per_character`` when validating session IDs.
- Fixed a bug (#4871) - :doc:`Query Builder <database/query_builder>` method ``update_batch()`` didn't properly handle identifier escaping.
- Fixed a bug (#4884) - :doc:`Query Builder <database/query_builder>` didn't properly parse field names ending in 'is' when used inside WHERE and HAVING statements.
-- Fixed a bug where ``CI_Log``, ``CI_Output``, ``CI_Email`` and ``CI_Zip`` didn't handle strings in a byte-safe manner when ``mbstring.func_override`` is enabled.
+- Fixed a bug where ``CI_Log``, ``CI_Output``, ``CI_Email`` and ``CI_Zip`` didn't handle strings in a byte-safe manner when ``mbstring.func_overload`` is enabled.
Version 3.1.1
=============
@@ -119,7 +126,7 @@
- Fixed a bug where :doc:`Query Builder <database/query_builder>` method ``insert_batch()`` tried to execute an unsupported SQL query with the 'ibase' and 'pdo/firebird' drivers.
- Fixed a bug (#4809) - :doc:`Database <database/index>` driver 'pdo/mysql' didn't turn off ``AUTOCOMMIT`` when starting a transaction.
- Fixed a bug (#4822) - :doc:`CAPTCHA Helper <helpers/captcha_helper>` didn't clear expired PNG images.
-- Fixed a bug (#4823) - :doc:`Session Library <libraries/sessions>` 'files' driver could enter an infinite loop if ``mbstring.func_override`` is enabled.
+- Fixed a bug (#4823) - :doc:`Session Library <libraries/sessions>` 'files' driver could enter an infinite loop if ``mbstring.func_overload`` is enabled.
- Fixed a bug (#4851) - :doc:`Database Forge <database/forge>` didn't quote schema names passed to its ``create_database()`` method.
- Fixed a bug (#4863) - :doc:`HTML Table Library <libraries/table>` method ``set_caption()`` was missing method chaining support.
- Fixed a bug (#4843) - :doc:`XML-RPC Library <libraries/xmlrpc>` client class didn't set a read/write socket timeout.