Sec Bug->Bug #73535 [Opn]: php_sockop_write() returns 0 on error, can be used to trigger Denial of Service

From: Date: Mon, 22 Jan 2018 20:57:18 +0000
Subject: Sec Bug->Bug #73535 [Opn]: php_sockop_write() returns 0 on error, can be used to trigger Denial of Service
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-213667@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=73535&edit=1

 ID:                 73535
 Updated by:         stas@php.net
 Reported by:        webmaster_20161114 at cubiclesoft dot com
 Summary:            php_sockop_write() returns 0 on error, can be used
                     to trigger Denial of Service
 Status:             Open
-Type:               Security
+Type:               Bug
 Package:            Streams related
 Operating System:   All
 PHP Version:        Irrelevant
 Block user comment: N
 Private report:     Y



Previous Comments:
------------------------------------------------------------------------
[2018-01-20 14:43:08] 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.

------------------------------------------------------------------------
[2017-09-07 16:17:44] 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>

------------------------------------------------------------------------
[2016-11-16 09:37:35] 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.

------------------------------------------------------------------------
[2016-11-16 06:11:08] 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 04:29:32] webmaster_20161114 at cubiclesoft dot com

CVE-2016-9321

------------------------------------------------------------------------


The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at

    https://bugs.php.net/bug.php?id=73535


--
Edit this bug report at https://bugs.php.net/bug.php?id=73535&edit=1


Thread (1 message)

  • stas@php.net
  • Unknown Message
    • stas@php.net
« previous php.bugs (#213667) next »