ID: 37496
Updated by: [EMAIL PROTECTED]
Reported By: gacek at intertele dot pl
-Status: Open
+Status: Assigned
Bug Type: CGI related
Operating System: Linux
PHP Version: 5CVS-2006-05-18 (snap)
Assigned To: dmitry
Previous Comments:
------------------------------------------------------------------------
[2006-05-24 12:44:06] gacek at intertele dot pl
Unfortunately your fix introduced another bug:
"limit" variable goes negative if free buffer length <
sizeof(fcgi_header) and req->out_hdr is NULL.
memcpy crashes, when copying region with negative length.
Change condition:
} else if (len - limit < sizeof(req->out_buf) - sizeof(fcgi_header)) {
to:
} else if (limit > 0 && len - limit < sizeof(req->out_buf) -
sizeof(fcgi_header)) {
------------------------------------------------------------------------
[2006-05-24 09:12:16] gacek at intertele dot pl
Bug has gone.
------------------------------------------------------------------------
[2006-05-22 09:27:52] [EMAIL PROTECTED]
The patch is partially commited into HEAD, PHP_5_2 and PHP_5_1. Did bug
go away?
------------------------------------------------------------------------
[2006-05-22 07:03:47] gacek at intertele dot pl
Actually, these are two bugs:
1. Interrupted flush_cgi() does not reset request->out_pos.
Thus we must check flush_cgi return code and stop filling buffer if
flush_cgi fails.
2. In fcgi_request structure there is additional "reserve" allocated
after out_buf. It's of sizeof(fcgi_header)+sizeof(_fcgi_end_reuest)
afair.
But at certain fcgi_write str lengths, there were three consecutive
operations on buffer:
close_packet() - ads 8-byte padding
open_packet() - ads header
fcgi_flush() - calls close_packet(), which ads 8-byte padding again
Last call might overrun buffer+reserve up to 7 bytes, smashing env
table pointer.
Changing first close_packet to fcgi_flush() eliminates this risk.
Patch against 5.1.4 below:
diff -ru php-5.1.4/sapi/cgi/fastcgi.c
php-5.1.4-patched/sapi/cgi/fastcgi.c
--- php-5.1.4/sapi/cgi/fastcgi.c 2006-05-03 17:39:16.000000000
+0200
+++ php-5.1.4-patched/sapi/cgi/fastcgi.c 2006-05-22
08:40:56.000000000 +0200
@@ -813,7 +813,10 @@
}
memcpy(req->out_pos, str, limit);
req->out_pos += limit;
- fcgi_flush(req, 0);
+ if (!fcgi_flush(req, 0)) {
+ req->keep = 0;
+ return -1;
+ }
open_packet(req, type);
memcpy(req->out_pos, str + limit, len - limit);
req->out_pos += len - limit;
@@ -821,12 +824,19 @@
int pos = 0;
int pad;
- close_packet(req);
+ if (!fcgi_flush(req, 0)) {
+ req->keep = 0;
+ return -1;
+ }
+
while ((len - pos) > 0xffff) {
open_packet(req, type);
fcgi_make_header(req->out_hdr, type, req->id,
0xfff8);
req->out_hdr = NULL;
- fcgi_flush(req, 0);
+ if (!fcgi_flush(req, 0)) {
+ req->keep = 0;
+ return -1;
+ }
if (safe_write(req, str + pos, 0xfff8) !=
0xfff8) {
req->keep = 0;
return -1;
@@ -840,7 +850,10 @@
open_packet(req, type);
fcgi_make_header(req->out_hdr, type, req->id, (len -
pos) - rest);
req->out_hdr = NULL;
- fcgi_flush(req, 0);
+ if (!fcgi_flush(req, 0)) {
+ req->keep = 0;
+ return -1;
+ }
if (safe_write(req, str + pos, (len - pos) - rest) !=
(len - pos) - rest) {
req->keep = 0;
return -1;
------------------------------------------------------------------------
[2006-05-18 13:01:29] gacek at intertele dot pl
Description:
------------
FastCGI buffer output (request->out_buf) is overwritten.
Reproduce code:
---------------
Not easily reproducible, but happens several times per hour on our
servers. Have multiple corefiles, all generated by this bug.
Actual result:
--------------
Core analysis (5.2 snapshot downloaded today):
(gdb) bt
#0 zend_hash_destroy (ht=0x7fffffd50f88) at
/usr/src/debug/php-5.1.4/Zend/zend_hash.c:519
#1 0x00000000005e95c2 in fcgi_finish_request (req=0x7fffffd4ef50) at
/usr/src/debug/php-5.1.4/sapi/cgi/fastcgi.c:600
#2 0x00000000005e9d1e in fcgi_accept_request (req=0x7fffffd4ef50) at
/usr/src/debug/php-5.1.4/sapi/cgi/fastcgi.c:630
#3 0x00000000005ebaf3 in main (argc=3, argv=0x7fffffd51348) at
/usr/src/debug/php-5.1.4/sapi/cgi/cgi_main.c:1334
(gdb) up 3
#3 0x00000000005ebaf3 in main (argc=3, argv=0x7fffffd51348) at
/usr/src/debug/php-5.1.4/sapi/cgi/cgi_main.c:1334
1334 while (!fastcgi ||
fcgi_accept_request(&request) >= 0) {
(gdb) p request
$5 = {listen_socket = 0, fd = 6, id = 1, keep = 0, in_len = 0, in_pad =
0, out_hdr = 0x0, out_pos = 0x7fffffd51bb0 "\001\003",
out_buf =
"\001\006\000\001\037ř\000\000Őîr\227\215śMĂć^\204W?uqóőÉ=\001ţu*Mť\024É<Ŕps\201OfR\234ă?Î\215ÉZîQ\221\035\225\212ő5\"Üż\236rŕ\214c§J$\223EĂBú\\
\206<\220NxŚÍf\216ŚDČ
W\făgc2M\025â\213ä\031\\qŢ\230,q\206Î\030\036qN6dĘ--\n7Č\020\036ĹExç\211'\027\210\024üŰ\233\030âľ3\223vł9KM\034K*Í°yĘâ˝?HŐ×ÉRĂý\222=ë´óÚľŮŃÁ\"ˇ@\203=Şú]ĽÍČXÇSVôÔ\205\207¨é\221."...,
reserved = "\001\006\000\001\f&\002\000\220E&ě?#Ľ\027", env
= {nTableSize = 3221323880, nTableMask = 1456291101,
nNumOfElements = 965277051, nNextFreeElement = 4251007554600414553,
pInternalPointer = 0xe4ef03c50554f8b,
pListHead = 0xaa3ab31e557c6d29, pListTail = 0x42a9064f1b27134e,
arBuckets = 0xdd29a8e682995fa, pDestructor = 0x9268853d0880b8af,
persistent = 205 'Í', nApplyCount = 32 ' ', bApplyProtection = 29
'\035'}}
(gdb) p &request->out_buf
$1 = (unsigned char (*)[8192]) 0x7fffffd4ef78
As we can see, request->out_pos is far more beyond request->out_buf+8k
(about 3k). Env pointer table got smashed.
Another core:
(gdb) where
#0 0x00002aaaac0732d0 in memcpy () from /lib64/libc.so.6
#1 0x00000000005e9702 in fcgi_write (req=0x7ffffff7e220, type=Variable
"type" is not available.
) at /usr/include/bits/string3.h:51
#2 0x00000000005ea683 in sapi_cgibin_ub_write (str=Variable "str" is
not available.
) at /usr/src/debug/php-5.1.4/sapi/cgi/cgi_main.c:239
#3 0x000000000053d944 in php_ub_body_write_no_header (str=Variable
"str" is not available.
) at /usr/src/debug/php-5.1.4/main/output.c:683
#4 0x000000000053e039 in php_end_ob_buffer (send_buffer=1 '\001',
just_flush=1 '\001') at /usr/src/debug/php-5.1.4/main/output.c:300
#5 0x000000000053e830 in php_b_body_write (
str=0x12d8988 "<tr><td class=\"spaceRow\" colspan=\"2\"
height=\"1\"><img src=\"templates/subSilver/images/spacer.gif\"
alt=\"\" width=\"1\" height=\"1\" /></td></tr>", str_length=137) at
/usr/src/debug/php-5.1.4/main/output.c:610
#6 0x00000000005699b8 in zend_print_zval_ex (write_func=0x5300a7
<php_body_write_wrapper>, expr=0x7ffffff77d28, indent=Variable "indent"
is not available.
)
at /usr/src/debug/php-5.1.4/Zend/zend.c:302
#7 0x0000000000585e08 in ZEND_ECHO_SPEC_TMP_HANDLER
(execute_data=0x7ffffff79fa0) at
/usr/src/debug/php-5.1.4/Zend/zend_vm_execute.h:3833
#8 0x0000000000581a2b in execute (op_array=0x12c83e8) at
/usr/src/debug/php-5.1.4/Zend/zend_vm_execute.h:92
#9 0x00000000005a4d1b in ZEND_INCLUDE_OR_EVAL_SPEC_CV_HANDLER
(execute_data=0x7ffffff7a480)
at /usr/src/debug/php-5.1.4/Zend/zend_vm_execute.h:19527
#10 0x0000000000581a2b in execute (op_array=0x10f9598) at
/usr/src/debug/php-5.1.4/Zend/zend_vm_execute.h:92
#11 0x0000000000581c81 in zend_do_fcall_common_helper_SPEC
(execute_data=0x7ffffff7b1a0)
at /usr/src/debug/php-5.1.4/Zend/zend_vm_execute.h:234
#12 0x0000000000581a2b in execute (op_array=0x10f9ff8) at
/usr/src/debug/php-5.1.4/Zend/zend_vm_execute.h:92
#13 0x0000000000581c81 in zend_do_fcall_common_helper_SPEC
(execute_data=0x7ffffff7bd80)
at /usr/src/debug/php-5.1.4/Zend/zend_vm_execute.h:234
#14 0x0000000000581a2b in execute (op_array=0x1074a98) at
/usr/src/debug/php-5.1.4/Zend/zend_vm_execute.h:92
#15 0x0000000000568ebf in zend_execute_scripts (type=8, retval=Variable
"retval" is not available.
) at /usr/src/debug/php-5.1.4/Zend/zend.c:1109
#16 0x0000000000531558 in php_execute_script
(primary_file=0x7ffffff80450) at
/usr/src/debug/php-5.1.4/main/main.c:1732
#17 0x00000000005eba62 in main (argc=3, argv=0x7ffffff80618) at
/usr/src/debug/php-5.1.4/sapi/cgi/cgi_main.c:1613
1613 php_execute_script(&file_handle
TSRMLS_CC);
(gdb) p request
$1 = {listen_socket = 0, fd = 3, id = 1, keep = 0, in_len = 0, in_pad =
0, out_hdr = 0x7ffffff80248,
out_pos = 0x7ffffff807a5 " </td>\n", ' ' <repeats 15 times>, "<td
valign=\"top\" align=\"left\" width=\"177\">\n", ' ' <repeats 18
times>, "<table border=\"0\" cellpadding=\"0\" cellspacing=\"0\"
style=\"border-collapse: collapse\">\n", ' ' <repeats 21 times>,
"<tr>\n "...,
out_buf = "\001\006\000\001\037ř\000\000\"></span><span
class=\"gensmall\"></span></td>\n", ' ' <repeats 12 times>, "</tr>\n
</table>\n </td>\n </tr>\n <tr>\n <td class=\"row2\"
width=\"150\" align=\"left\" valign=\"middle\">\n", ' ' <repeats 12
times>, "<a href=\""..., reserved = "\000\006\000\000\000\000\000\000
", env = {nTableSize = 1047688252, nTableMask = 538976266,
nNumOfElements = 538976288, nNextFreeElement = 2334118308470595616,
pInternalPointer = 0x3d6e6170736c6f63,
pListHead = 0x696c617620223222, pListTail = 0x74746f62223d6e67,
arBuckets = 0x6170733c3e226d6f, pDestructor = 0x3d7373616c63206e,
persistent = 34 '"', nApplyCount = 112 'p', bApplyProtection = 111
'o'}}
(gdb) p &request->out_buf
$2 = (unsigned char (*)[8192]) 0x7ffffff7e248
Again, request->out_pos > &request->out_buf+8k (about 1.5k).
Same bug is in stable 5.1.4 version.
------------------------------------------------------------------------
--
Edit this bug report at http://bugs.php.net/?id=37496&edit=1
