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

 ID:                 64187
 User updated by:    nachms+php at gmail dot com
 Reported by:        nachms+php at gmail dot com
 Summary:            CGI/FastCGI truncates input to modulo 4GB
 Status:             Open
 Type:               Bug
 Package:            Streams related
 Operating System:   Linux
 PHP Version:        5.4.11
 Block user comment: N
 Private report:     N

 New Comment:

Problems in PHP are also a bit larger than I described here, although perhaps 
should be filed as a separate bug.

32-bit OSs generally have "large file support", and can support handling data 
at much larger than 4GB. On most UNIXs, getconf can indicate appropriate flags 
to enable such support. On Windows, large file support is always available.

Ideally PHP should ensure such support is available and properly used. For 
starters, Content-Length header is stored within a long. It should be stored in 
a type guaranteed to be 64 bits, and not depend if the system itself is 32 or 
64 bit.

It is okay to limit the amount of data that can be read at once is limited to 
32-bit, even on a 64-bit platform. But the overall size on files or input 
streams from pipes and sockets should not be.


Previous Comments:
------------------------------------------------------------------------
[2013-02-19 02:31:22] payden at paydensutherland dot com

Oh, I'm sorry.  I must have misread it before.  I see you're not ignoring 
count_bytes.  You're just taking out the MIN() on count_bytes, and remaining 
data 
to be read.  Let me keep my mouth shut until I come up with something 
intelligent 
to say.  :)

------------------------------------------------------------------------
[2013-02-19 02:27:04] payden at paydensutherland dot com

I may be way off here, but from what I can see in SAPI.c (for 5.4.11, line 266 
is 
where the callback is invoked), count_bytes is the number of bytes that the 
sapi_module_struct->read_post callback can safely stuff in the buffer without 
overflowing its bounds.  I think ignoring count_bytes in the callback is 
probably 
a bad idea.  Just my two cents.  I'll be looking more into it and I'll post 
here 
if I come up with a solution.

------------------------------------------------------------------------
[2013-02-11 19:27:39] nachms+php at gmail dot com

Due to lack of comments as mentioned above, I'm unsure what the problematic 
loop is needed for. However thinking more about it, perhaps it's needed for 
pipelining or multiplexing?

In which case, changing read_post_bytes in SAPI.h from int to long, and 
removing the uint cast from SG(request_info).content_length may be the correct 
solution.

------------------------------------------------------------------------
[2013-02-11 10:11:29] nachms+php at gmail dot com

In cgi_main.c, in sapi_cgi_read_post() and sapi_fcgi_read_post(), I found if I 
comment out the line: count_bytes = MIN(count_bytes, (uint) 
SG(request_info).content_length - SG(read_post_bytes));
Then the bug is fixed.

I'm not sure why the amount of bytes going to be read is bounded to 
read_post_bytes subtracted from the length. read() will only read the amount 
that it can, and it doesn't matter if you asked for too much, it will return 
what is available.

For FastCGI, I'm not familiar enough with fcgi_read() enough to know if the 
lack of bounding causes a problem or not. However, mod_php5 as an example 
doesn't bound it. And if bounding is needed, this code needs to make use of 
long or unsigned long instead of int and unit.

------------------------------------------------------------------------
[2013-02-11 02:37:29] nachms+php at gmail dot com

I think I found the bug in cgi_main.c:

static int sapi_cgi_read_post(char *buffer, uint count_bytes TSRMLS_DC)
{
        uint read_bytes = 0;
        int tmp_read_bytes;

        count_bytes = MIN(count_bytes, (uint) SG(request_info).content_length - 
SG(read_post_bytes));
        while (read_bytes < count_bytes) {
                tmp_read_bytes = read(STDIN_FILENO, buffer + read_bytes, 
count_bytes - read_bytes);
                if (tmp_read_bytes <= 0) {
                        break;
                }
                read_bytes += tmp_read_bytes;
        }
        return read_bytes;
}

It looks like content_length, a long, is being truncated to a uint. I'll look 
into fixing this based on what mod_php5 does.

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


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=64187


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

Reply via email to