Fix #2822: Incorrect usage of fwrite()

We only used to check (and not always) if the return value of fwrite() is boolean FALSE,
while it is possible that the otherwise returned bytecount is less than the length of
data that we're trying to write. This allowed incomplete writes over network streams
and possibly a few other edge cases.

7 files changed, 81 insertions, 21 deletions

diff --git a/system/core/Log.php b/system/core/Log.php
index ff3c63568..63fef2088 100644
--- a/system/core/Log.php
+++ b/system/core/Log.php
@@ -178,7 +178,15 @@ class CI_Log {
 		$message .= $level.' - '.date($this->_date_fmt).' --> '.$msg."\n";
 
 		flock($fp, LOCK_EX);
-		fwrite($fp, $message);
+
+		for ($written = 0, $length = strlen($message); $written < $length; $written += $result)
+		{
+			if (($result = fwrite($fp, substr($message, $written))) === FALSE)
+			{
+				break;
+			}
+		}
+
 		flock($fp, LOCK_UN);
 		fclose($fp);
 
@@ -187,7 +195,7 @@ class CI_Log {
 			@chmod($filepath, FILE_WRITE_MODE);
 		}
 
-		return TRUE;
+		return is_int($result);
 	}
 
 }
diff --git a/system/core/Output.php b/system/core/Output.php
index 10332f0d8..a7680b3d0 100644
--- a/system/core/Output.php
+++ b/system/core/Output.php
@@ -542,17 +542,26 @@ class CI_Output {
 			return;
 		}
 
-		$expire = time() + ($this->cache_expiration * 60);
-
-		// Put together our serialized info.
-		$cache_info = serialize(array(
-			'expire'	=> $expire,
-			'headers'	=> $this->headers
-		));
-
 		if (flock($fp, LOCK_EX))
 		{
-			fwrite($fp, $cache_info.'ENDCI--->'.$output);
+			$expire = time() + ($this->cache_expiration * 60);
+
+			// Put together our serialized info.
+			$cache_info = serialize(array(
+				'expire'	=> $expire,
+				'headers'	=> $this->headers
+			));
+
+			$output = $cache_info.'ENDCI--->'.$output;
+
+			for ($written = 0, $length = strlen($output); $written < $length; $written += $result)
+			{
+				if (($result = fwrite($fp, substr($output, $written))) === FALSE)
+				{
+					break;
+				}
+			}
+
 			flock($fp, LOCK_UN);
 		}
 		else
@@ -560,13 +569,22 @@ class CI_Output {
 			log_message('error', 'Unable to secure a file lock for file at: '.$cache_path);
 			return;
 		}
+
 		fclose($fp);
-		@chmod($cache_path, FILE_WRITE_MODE);
 
-		log_message('debug', 'Cache file written: '.$cache_path);
+		if (is_int($result))
+		{
+			@chmod($cache_path, FILE_WRITE_MODE);
+			log_message('debug', 'Cache file written: '.$cache_path);
 
-		// Send HTTP cache-control headers to browser to match file cache settings.
-		$this->set_cache_header($_SERVER['REQUEST_TIME'], $expire);
+			// Send HTTP cache-control headers to browser to match file cache settings.
+			$this->set_cache_header($_SERVER['REQUEST_TIME'], $expire);
+		}
+		else
+		{
+			@unlink($cache_path);
+			log_message('error', 'Unable to write the complete cache content at: '.$cache_path);
+		}
 	}
 
 	// --------------------------------------------------------------------
diff --git a/system/helpers/file_helper.php b/system/helpers/file_helper.php
index 4b45a62d0..0587740b1 100644
--- a/system/helpers/file_helper.php
+++ b/system/helpers/file_helper.php
@@ -79,11 +79,19 @@ if ( ! function_exists('write_file'))
 		}
 
 		flock($fp, LOCK_EX);
-		fwrite($fp, $data);
+
+		for ($written = 0, $length = strlen($data); $written < $length; $written += $result)
+		{
+			if (($result = fwrite($fp, substr($data, $written))) === FALSE)
+			{
+				break;
+			}
+		}
+
 		flock($fp, LOCK_UN);
 		fclose($fp);
 
-		return TRUE;
+		return is_int($result);
 	}
 }
 
diff --git a/system/libraries/Email.php b/system/libraries/Email.php
index 9487ad486..f4efff882 100644
--- a/system/libraries/Email.php
+++ b/system/libraries/Email.php
@@ -2097,7 +2097,16 @@ class CI_Email {
 	 */
 	protected function _send_data($data)
 	{
-		if (fwrite($this->_smtp_connect, $data.$this->newline) === FALSE)
+		$data .= $this->newline;
+		for ($written = 0, $length = strlen($data); $written < $length; $written += $result)
+		{
+			if (($result = fwrite($this->_smtp_connect, substr($data, $written))) === FALSE)
+			{
+				break;
+			}
+		}
+
+		if ($result === FALSE)
 		{
 			$this->_set_error_message('lang:email_smtp_data_failure', $data);
 			return FALSE;
diff --git a/system/libraries/Xmlrpc.php b/system/libraries/Xmlrpc.php
index 1f93e6981..ab907e706 100644
--- a/system/libraries/Xmlrpc.php
+++ b/system/libraries/Xmlrpc.php
@@ -724,7 +724,15 @@ class XML_RPC_Client extends CI_Xmlrpc
 			.'Content-Length: '.strlen($msg->payload).$r.$r
 			.$msg->payload;
 
-		if (fwrite($fp, $op, strlen($op)) === FALSE)
+		for ($written = 0, $length = strlen($op); $written < $length; $written += $result)
+		{
+			if (($result = fwrite($fp, substr($op, $written))) === FALSE)
+			{
+				break;
+			}
+		}
+
+		if ($result === FALSE)
 		{
 			error_log($this->xmlrpcstr['http_error']);
 			return new XML_RPC_Response(0, $this->xmlrpcerr['http_error'], $this->xmlrpcstr['http_error']);
diff --git a/system/libraries/Zip.php b/system/libraries/Zip.php
index 250ee02cd..b10b0bb0f 100644
--- a/system/libraries/Zip.php
+++ b/system/libraries/Zip.php
@@ -403,11 +403,19 @@ class CI_Zip {
 		}
 
 		flock($fp, LOCK_EX);
-		fwrite($fp, $this->get_zip());
+
+		for ($written = 0, $data = $this->get_zip(), $length = strlen($data); $written < $length; $written += $result)
+		{
+			if (($result = fwrite($fp, substr($data, $written))) === FALSE)
+			{
+				break;
+			}
+		}
+
 		flock($fp, LOCK_UN);
 		fclose($fp);
 
-		return TRUE;
+		return is_int($result);
 	}
 
 	// --------------------------------------------------------------------
diff --git a/user_guide_src/source/changelog.rst b/user_guide_src/source/changelog.rst
index 77378521b..a1dc3c1d1 100644
--- a/user_guide_src/source/changelog.rst
+++ b/user_guide_src/source/changelog.rst
@@ -669,6 +669,7 @@ Bug fixes for 3.0
 -  Fixed a bug (#346) - with ``$config['global_xss_filtering']`` turned on, the ``$_GET``, ``$_POST``, ``$_COOKIE`` and ``$_SERVER`` superglobals were overwritten during initialization time, resulting in XSS filtering being either performed twice or there was no possible way to get the original data, even though options for this do exist.
 -  Fixed an edge case (#555) - incorrect browser version was reported for Opera 10+ due to a non-standard user-agent string.
 -  Fixed a bug (#133) - :doc:`Text Helper <helpers/text_helper>` :func:`ascii_to_entities()` stripped the last character if it happens to be in the extended ASCII group.
+-  Fixed a bug (#2822) - ``fwrite()`` was used incorrectly throughout the whole framework, allowing incomplete writes when writing to a network stream and possibly a few other edge cases.
 
 Version 2.1.4
 =============