#49286 [Ver]: php://input (php_stream_input_read) is broken

ID: 49286 User updated by: jost_boekemeier at users dot sf dot net Reported By: jost_boekemeier at users dot sf dot net Status: Verified Bug Type: Filesystem function related Operating System: * PHP Version: 5.*, 6 (2009-08-20) New Comment: If you look the comment in the source code and then look at what the code does, you'll immediately see that the code doesn't do what the comment says it does. Instead of looking at the source to verify what I've said, you have forced me to write a test case, which is something that YOU should do. Sorry, but if you threat all reports like this, you'll miss 90% of all real bugs. Previous Comments: ------------------------------------------------------------------------ [2009-08-20 11:30:33] jani@php.net Thanks for FINALLY providing the test case. It's not about being able to read code, it's about getting dozens of bogus reports daily and interpreting which are really valid AND not yet fixed in repo. You failed in so many ways when you reported this bug that in your shoes, I'd be a little less arrogant. Chances to getting anything fixed are much higher with little bit of respect.. ------------------------------------------------------------------------ [2009-08-20 09:59:24] jost_boekemeier at users dot sf dot net A simple test case (for those who can't read code): REDIRECT_STATUS="200" CONTENT_TYPE="application/x-www-form-urlencoded" SCRIPT_FILENAME="test.php" REQUEST_METHOD="POST" GATEWAY_INTERFACE="CGI/1.1" CONTENT_LENGTH="1" strace /usr/bin/php-cgi.bin </dev/zero <?php $file = fopen("php://input", "r"); $str = fread($file, 1024); fclose($file); ?> => read(0, "\0"..., 1) = 1 ... read(3, "<?php\n$file = fopen(\"php://input\""..., 4096) = 88 _llseek(3, 0, [0], SEEK_SET) = 0 setitimer(ITIMER_PROF, {it_interval={0, 0}, it_value={30, 0}}, NULL) = 0 rt_sigaction(SIGPROF, {0x81ff8c0, [PROF], SA_RESTART}, {0x81ff8c0, [PROF], SA_RESTART}, 8) = 0 rt_sigprocmask(SIG_UNBLOCK, [PROF], NULL, 8) = 0 ioctl(3, SNDCTL_TMR_TIMEBASE or TCGETS, 0xbfd94038) = -1 ENOTTY (Inappropriate ioctl for device) read(3, "<?php\n$file = fopen(\"php://input\""..., 8192) = 88 read(3, ""..., 4096) = 0 read(3, ""..., 8192) = 0 close(3) = 0 write(1, "X-Powered-By: PHP/5.2.6"..., 23X-Powered-By: PHP/5.2.6) = 23 write(1, "\r\n"..., 2 ) = 2 write(1, "Content-type: text/html"..., 23Content-type: text/html) = 23 write(1, "\r\n"..., 2 ) = 2 write(1, "\r\n"..., 2 ) = 2 write(1, "\n\n"..., 2 ) = 2 read(0, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 3999) = 3999 read(0, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 3999) = 3999 read(0, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 3999) = 3999 .... ------------------------------------------------------------------------ [2009-08-19 12:37:05] jost_boekemeier at users dot sf dot net I cannot provide one. This bug has been reported by someone else. I have only looked at the PHP code, noticed a bug in the php://input function and reported it along with a patch to fix it. You can see the bug by looking at the source code: If you post 10 bytes, the post handler will consume these bytes and set the global "read_post_bytes" to 10. On shutdown some sapi (cgi, fastcgi) try to consume the difference between read_post_bytes and CONTENT_LENGTH . As. Php://input consumes bytes from an internal buffer but incorrectly increments read_post_bytes in any case, pushing it over the CONTENT_LENGTH limit. sapi shutdown tries to read (unsigned)10-20 = 2^32-10 bytes. This results in an unnecessary read() kernel call. This causes a short delay but it is not fatal unless someone uses a persistent connection and cannot signal an explicit EOF. Any SAPI which requires that PHP reads no more content than CONTENT_LENGTH will crash or hang when php://input is used. Others are slow, but they will not crash. ------------------------------------------------------------------------ [2009-08-19 11:44:53] jani@php.net First of all: Not all people follow all the mailing lists in the world. You need to provide a test case before we start applying any patches. So far you haven't shown any bug here. ------------------------------------------------------------------------ [2009-08-19 06:39:49] jost_boekemeier at users dot sf dot net diff -u /home/jost/php-5.2.5/ext/standard/php_fopen_wrapper.c\~ /home/jost/php- 5.2.5/ext/standard/php_fopen_wrapper.c --- /home/jost/php-5.2.5/ext/standard/php_fopen_wrapper.c~ 2007-10-04 15:31:11.000000000 +0200 +++ /home/jost/php-5.2.5/ext/standard/php_fopen_wrapper.c 2009-08-19 08:26:01.000000000 +0200 @@ -78,7 +78,7 @@ if(!stream->eof) { if(SG(request_info).raw_post_data) { /* data has already been read by a post handler */ - read_bytes = SG(request_info).raw_post_data_length - *position; + size_t read_bytes = SG(request_info).raw_post_data_length - *position; if(read_bytes <= count) { stream->eof = 1; } else { @@ -86,7 +86,9 @@ } if(read_bytes) { memcpy(buf, SG(request_info).raw_post_data + *position, read_bytes); + *position += read_bytes; } + return read_bytes; } else if(sapi_module.read_post) { read_bytes = sapi_module.read_post(buf, count TSRMLS_CC); if(read_bytes <= 0){ Diff finished. Wed Aug 19 08:26:08 2009 ------------------------------------------------------------------------ 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 http://bugs.php.net/49286 -- Edit this bug report at http://bugs.php.net/?id=49286&edit=1