ID: 27383
User updated by: remijnj at eidetica dot com
Reported By: remijnj at eidetica dot com
-Status: Closed
+Status: Open
Bug Type: HTTP related
Operating System: Linux (Slackware 9.1)
PHP Version: 4.3.5RC3
New Comment:
I reopened the bug because the tmp_line is still not initialized. See
my previous comment (forgot to open the bug again) which has a simple
patch from cvs diff -u.
Joost
Previous Comments:
------------------------------------------------------------------------
[2004-02-24 17:51:34] remijnj at eidetica dot com
Thanks for adding it to CVS so quickly. I got the cvs version checked
out now and i notice one important bit missing (because i didn't
explain it properly i guess).
My initial problem (garbage bytes in "HTTP request failed" error) is
still there as far as i can see from the code. Let me explain.
The line :
php_stream_wrapper_log_error(wrapper, options TSRMLS_CC, "HTTP request
failed! %s", tmp_line);
can be reached with tmp_line uninitialized. That is why i added the
'tmp_line[0] = '\0';' at the start of the function. Here an incremental
patch on top of this.
Index: ext/standard/http_fopen_wrapper.c
===================================================================
RCS file: /repository/php-src/ext/standard/http_fopen_wrapper.c,v
retrieving revision 1.53.2.15
diff -u -r1.53.2.15 http_fopen_wrapper.c
--- ext/standard/http_fopen_wrapper.c 24 Feb 2004 21:53:56 -0000
1.53.2.15
+++ ext/standard/http_fopen_wrapper.c 24 Feb 2004 22:51:56 -0000
@@ -107,6 +107,8 @@
size_t chunk_size = 0, file_size = 0;
int eol_detect, have_header = 0;
+ tmp_line[0] = '\0';
+
if (redirect_max < 1) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "Circular
redirect, aborting.");
return NULL;
------------------------------------------------------------------------
[2004-02-24 16:46:30] iliaa@php.net
This bug has been fixed in CVS.
Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.
Thank you for the report, and for helping us make PHP better.
------------------------------------------------------------------------
[2004-02-24 14:19:26] remijnj at eidetica dot com
Here the patch which fixes it. I hope i've done it in the right patch
format (diff -urN).
diff -urN php-4.3.5RC3/ext/standard/http_fopen_wrapper.c
php-4.3.5RC3-mine/ext/standard/http_fopen_wrapper.c
--- php-4.3.5RC3/ext/standard/http_fopen_wrapper.c 2003-11-28
19:51:14.000000000 +0100
+++ php-4.3.5RC3-mine/ext/standard/http_fopen_wrapper.c 2004-02-24
19:51:07.000000000 +0100
@@ -107,6 +107,7 @@
size_t chunk_size = 0, file_size = 0;
int eol_detect, have_header = 0;
+ tmp_line = '\0';
if (redirect_max < 1) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "Circular redirect,
aborting.");
return NULL;
@@ -345,11 +346,24 @@
if (php_stream_gets(stream, tmp_line, sizeof(tmp_line)-1) !=
NULL) {
zval *http_response;
int response_code;
+ int tmp_line_len;
+
+ tmp_line_len = strlen(tmp_line);
MAKE_STD_ZVAL(http_response);
ZVAL_NULL(http_response);
- response_code = atoi(tmp_line + 9);
+ if (tmp_line_len > 9) {
+ response_code = atoi(tmp_line + 9);
+ } else {
+ /*
+ * short http_response, if not caught like
+ * this we'd pass uninitialized memory to
+ * atoi (SEGV if there is no '\0' byte in
+ * there)
+ */
+ response_code = 0;
+ }
switch(response_code) {
case 200:
case 302:
@@ -365,7 +379,7 @@
tmp_line, response_code);
}
- Z_STRLEN_P(http_response) = strlen(tmp_line);
+ Z_STRLEN_P(http_response) = tmp_line_len;
Z_STRVAL_P(http_response) = estrndup(tmp_line,
Z_STRLEN_P(http_response));
if (Z_STRVAL_P(http_response)[Z_STRLEN_P(http_response)-1]=='\n')
{
Z_STRVAL_P(http_response)[Z_STRLEN_P(http_response)-1]=0;
------------------------------------------------------------------------
[2004-02-24 14:13:53] remijnj at eidetica dot com
Description:
------------
I have seen uninitialized memory being printed out in my php eror log.
Some of the errors are like:
[24-Feb-2004 12:00:12] PHP Warning: file_get_contents(<snip>) failed
to open stream: HTTP request
failed! ??B^P in /usr/local/www/include/file.inc on line 17
This happened when the apache server i connected to was too busy to
handle the request (load way too high).
In that specific case tmp_line will be used uninitialized. This code
could possibly lead to a SEGV (Segmentation Violation).
Looking at the code i also spotted another (more unlikely) bug which
could also result in a SEGV.
I have prepared a patch against 4.3.5RC5 which should solve this
problem (in my opinion).
If anyone has any questions on this report or my patch (which i will
try to add later) please contact me.
------------------------------------------------------------------------
--
Edit this bug report at http://bugs.php.net/?id=27383&edit=1