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

Edit report at https://bugs.php.net/bug.php?id=73535&edit=1 ID: 73535 Updated by: bwoebi@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: Bug +Type: Documentation Problem Package: Streams related Operating System: All PHP Version: Irrelevant Block user comment: N Private report: N New Comment: 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. 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