|
|
php.net | support | documentation | report a bug | advanced search | search howto | statistics | random bug | login |
[2004-02-24 14:13 UTC] 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. PatchesPull RequestsHistoryAllCommentsChangesGit/SVN commits
|
|||||||||||||||||||||||||||
Copyright © 2001-2024 The PHP GroupAll rights reserved. |
Last updated: Wed Apr 24 22:01:30 2024 UTC |
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;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;