php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #73535 php_sockop_write() returns 0 on error, can be used to trigger Denial of Service
Submitted: 2016-11-16 04:27 UTC Modified: 2019-07-22 19:13 UTC
Votes:1
Avg. Score:4.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:0 (0.0%)
Same OS:0 (0.0%)
From: webmaster_20161114 at cubiclesoft dot com Assigned: nikic (profile)
Status: Closed Package: Streams related
PHP Version: Irrelevant OS: All
Private report: No CVE-ID: None
View Developer Edit
Welcome! If you don't have a Git account, you can't do anything here.
If you reported this bug, you can edit this bug over here.
(description)
Block user comment
Status: Assign to:
Package:
Bug Type:
Summary:
From: webmaster_20161114 at cubiclesoft dot com
New email:
PHP Version: OS:

 

 [2016-11-16 04:27 UTC] webmaster_20161114 at cubiclesoft dot com
Description:
------------
Premature socket termination from one side and then using fwrite() to send more data from the other side than the current maximum TCP packet size causes php_sockop_write() to emit a PHP Notice about a broken connection/pipe and return 0.  This, in turn, will lead to an infinite loop if fwrite() is used as seen in the documentation for fwrite().  Many implementations have utilized the PHP documentation's fwrite_stream() userland function in various forms for TCP/IP socket communications.

Attempting to detect network failure using other common functions does not work:  stream_select() immediately returns and indicates that the socket as both readable and writable ($except array, if passed, is empty) and feof() returns false.  Attempting to read a broken socket to trigger a change in a feof() call is not always an option.

An attacker has multiple vectors to maliciously use this knowledge to perform a Denial of Service attack, especially if they can command a host to communicate with a server that they control.  For example, an attacker could submit a very long URL of an image where the target host will then use a PHP userland library to go retrieve the URL using fwrite() in a loop to send all of the data.  Upon connecting and optionally reading a small amount of the input, the attacker's server prematurely terminates the request.  The PHP script then enters an infinite loop, occupying one CPU core until the script is terminated (e.g. timeout).  The situation can also arise due to normal network and software conditions (e.g. an Apache configuration might not allow a large payload for the request line).

Assuming that a return value of 0 from fwrite() is equal to an underlying network failure is not a valid solution for non-blocking and, to a lesser extent, blocking sockets.  In addition, it is currently extremely difficult to differentiate failure vs. a temporary condition (e.g. an internal buffer is still full or a slightly broken blocking sockets implementation) vs. attempting to write 0 bytes since 0 is returned in all three cases.  The latter case being rather minor while the former two are vital for non-blocking sockets to function properly.

The offending lines of code that are most concerning are these in php_sockop_write():

	if (didwrite < 0) {
		didwrite = 0;
	}

A possible fix could be to modify php_sockop_write():

-	if (didwrite < 0) {
-		didwrite = 0;
-	}

And then modify fwrite():

	ret = php_stream_write(stream, input, num_bytes);

+	if (ret < 0) {
+		RETURN_FALSE;
+	}

	RETURN_LONG(ret);

To keep BC breaks to a minimum and correctly return false on socket write errors.  Doing the above may also correct other write error issues as well, standardizing on false as a fwrite() $handle error condition.

There may be a similar problem lurking in OpenSSL enabled sockets with fwrite() in a loop.

I am in the process of requesting a CVE ID from MITRE for this vulnerability.

Test script:
---------------
Server:

<?php
	$fp = stream_socket_server("tcp://127.0.0.1:10000", $errornum, $errorstr, STREAM_SERVER_BIND | STREAM_SERVER_LISTEN);

	do
	{
		if (($fp2 = stream_socket_accept($fp)) !== false)
		{
			echo fread($fp2, 100000) . "\n";
			$str = str_repeat("S", 4096);
			fwrite($fp2, $str);
			sleep(3);
			exit();
		}
	} while (1);
?>


Client:

<?php
	$fp = stream_socket_client("tcp://127.0.0.1:10000", $errornum, $errorstr, 10, STREAM_CLIENT_CONNECT);

	// The following function was taken verbatim from the official example for fwrite():  http://php.net/manual/en/function.fwrite.php
	function fwrite_stream($fp, $string) {
		for ($written = 0; $written < strlen($string); $written += $fwrite) {
			$fwrite = fwrite($fp, substr($string, $written));

			if ($fwrite === false) {
				return $written;
			}
		}
		return $written;
	}

	// Set up the broken connection.
	$str = str_repeat("C", 100000);
	fwrite_stream($fp, $str);
	echo fread($fp, 4096) . "\n";
	sleep(6);

	// Connection is broken here.  Infinite loop in fwrite_stream().  fwrite() emits PHP Notices and the process has high CPU usage.
	fwrite_stream($fp, $str);
?>

Expected result:
----------------
fwrite() should return false and the example client should exit.

Actual result:
--------------
fwrite() returns 0 after the connection breaks.  As a result, the client enters into an infinite loop, each call to fwrite() emitting a line similar to this one:

PHP Notice:  fwrite(): send of 8192 bytes failed with errno=32 Broken pipe in /home/testuser/bug_client.php on line 7

Also, the CPU of one core is consumed until the PHP script is manually terminated.

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-11-16 04:29 UTC] webmaster_20161114 at cubiclesoft dot com
CVE-2016-9321
 [2016-11-16 06:11 UTC] stas@php.net
-Status: Open +Status: Feedback
 [2016-11-16 06:11 UTC] stas@php.net
Looks like bad documentation example, not sure why it's a security issue? Certainly don't see why this would have a CVE.
 [2016-11-16 09:37 UTC] webmaster_20161114 at cubiclesoft dot com
-Status: Feedback +Status: Open
 [2016-11-16 09:37 UTC] webmaster_20161114 at cubiclesoft dot com
Well it also took me a while to get from "documentation bug" to "remote exploit".  This is a remotely exploitable bug where a host that uses PHP fwrite() in a loop for network communications can DoS itself with a little bit of help, which is especially true for non-blocking sockets.  The behavior can be exploited (see the provided example) and stems from a bug in PHP core.  It warrants a CVE since I can't find a version of PHP released in the last decade without the bug and it affects PHP 7 as well.

For the most part, the code presented in the documentation (and code similar to it) will *appear* to work fine.  Copy-pasta software development at work here:  People have tried that code sample, seen that it "works", and are running it in production environments.  When it doesn't work as advertised sometime in the future, they'll eventually figure out that testing for 0 and just assuming the connection died will *appear* to work for blocking sockets in many cases.  However, there is no guarantee that a blocking sockets implementation won't return 0 for strange edge cases and prematurely return.  The net result is that random, unexplainable bugs will crop up even for blocking sockets.  Sure we can get partial mitigation by testing for 0 with blocking sockets, but good luck with replicating and solving the subsequent bugs that arise from doing so.

Even if blocking sockets can be partially mitigated, there is no workaround such as assuming 0 = broken connection for non-blocking sockets.  Just because a stream_select() call for a socket reports that a stream is writable does NOT guarantee that the underlying implementation of the stream is actually writable (this does happen).  That means fwrite() could simply return 0 even if stream_select() returned the socket in the $write array.  steam_select() only indicates that at that moment in time the socket *appears* to be writable.  The stream isn't necessarily dead if fwrite() returned 0 - or maybe it is - but there's no easy way to know which is which because error conditions are currently buried by the implementation.  As it is right now, the only legitimate option is to loop and consume CPU until some sort of socket timeout is exceeded, which assumes that someone thought that far ahead and was willing to burn in their CPU with a PHP loop that does nothing.

The problem extends to userland libraries that support both blocking and non-blocking sockets.  Such libraries tend to implement a state engine, which loops through the various states.  In other words, implicitly calling fwrite() in a loop.  Sure, the library can partially mitigate the problem for blocking sockets by testing for 0, subject to the aforementioned perfect blocking sockets implementation in an ideal world which doesn't exist.  However, even with a "fix" for blocking sockets applied, that same library's non-blocking sockets implementation is still broken in a way that can be exploited.

The source of this bug is that someone decided to completely ignore socket error states for socket write operations.  That's never the right thing to do.  This bug is very clearly NOT a documentation bug.  php_sockop_write()'s current behavior is wrong.  The simple solution is to let the error state return to fwrite() and then map it to RETURN_FALSE as the documentation says will happen and what userland developers expect to happen so they can properly terminate the socket connection and associated resources.  Doing those things happens to make the fwrite_stream() example correct while also fixing non-blocking socket servers and clients so they can correctly identify socket error states.
 [2017-09-07 16:17 UTC] cmb@php.net
Hm, your suggested patch wouldn't work, since php_sockop_write()
and php_stream_write() both return a size_t. Since the latter is
a PHP_API, we can't change that.

Not sure, what to do. :(

[1] <https://php-lxr.adamharvey.name/source/xref/PHP-7.2/main/streams/xp_socket.c#61>
 [2018-01-20 14:43 UTC] webmaster_20161114 at cubiclesoft dot com
I definitively ran into this issue this week in a live production environment.  When running PHP userland code as a server (aka non-blocking sockets bound to a port), this bug can be triggered fairly easily to take out the whole server.  To date I had only triggered the bug in testing but realized it could happen with any userland server written in PHP at any point in time.  As technology progresses, more people are writing servers in various languages, including PHP (e.g. ReactPHP).  Eventually someone else will independently discover this bug and they won't play nice.  Please prioritize a fix.

Also, someone please update this bug report with the CVE so that it gets noticed/triaged.
 [2018-01-22 20:57 UTC] stas@php.net
-Type: Security +Type: Bug
 [2018-07-10 15:07 UTC] bwoebi@php.net
-Type: Bug +Type: Documentation Problem
 [2018-07-10 15:07 UTC] bwoebi@php.net
I am aware of that behavior and I agree it's suboptimal.
The documentation should be updated to properly handle 0 bytes written (return value is 0). There generally are just two possible reasons why it would return 0 on an otherwise perfectly clean stream: a) the buffer is full or b) the other end closed their connection end. The former only applies for non-blocking streams, where, to distinguish between both cases you use stream_select() (or equivalents).

I wish as well for a refactoring of these APIs allowing for easier error handling. But it's not a bug nor a security issue inherent to PHP, an user can work around it.
 [2018-07-20 15:32 UTC] webmaster_20161114 at cubiclesoft dot com
-Type: Documentation Problem +Type: Bug
 [2018-07-20 15:32 UTC] webmaster_20161114 at cubiclesoft dot com
It's clearly NOT a documentation problem but a bug in PHP.  Please stop marking it as such.  stream_select() returns a writeable socket, so software will attempt to write, the fwrite() fails due to disconnect but returns 0 instead of false.  The code loops around to stream_select() on the socket again, which returns again immediately, fwrite() fails, etc.  As such, it's simply not possible to detect write failures with non-blocking sockets which means there is no userland workaround.  0 !== false.  Returning false from fwrite() is absolutely necessary.
 [2019-07-18 14:33 UTC] nikic@php.net
PR up at https://github.com/php/php-src/pull/4433. We should take the chance to fix this in 7.4 before we get too late in the release cycle.
 [2019-07-22 19:13 UTC] nikic@php.net
-Status: Open +Status: Closed -Assigned To: +Assigned To: nikic
 [2019-07-22 19:13 UTC] nikic@php.net
This should be fixed in PHP 7.4.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 12:01:29 2024 UTC