ID:               37496
 Updated by:       [EMAIL PROTECTED]
 Reported By:      gacek at intertele dot pl
-Status:           Assigned
+Status:           Closed
 Bug Type:         CGI related
 Operating System: Linux
 PHP Version:      5CVS-2006-05-18 (snap)
 Assigned To:      dmitry
 New Comment:

Thank you for your help.

Fixed in CVS HEAD, PHP_5_2 and PHP_5_1.


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&#345;\000\000&#336;îr\227\215&#347;M&#258;&#263;^\204W?uqó&#337;É=\001&#355;u*M&#357;\024É<&#340;ps\201OfR\234&#259;?Î\215ÉZîQ\221\035\225\212&#337;5\"Ü&#380;\236r&#341;\214c§J$\223E&#258;Bú\\
\206<\220Nx&#346;Íf\216&#346;D&#268;
W\f&#259;gcŽ2M\025â\213ä\031\\q&#354;\230,q\206Î\030\036qN6d&#280;--\n7&#268;\020\036&#313;Exç\211'\027\210\024ü&#368;\233\030­â&#318;3\223v&#322;9KM\034K*Í°yŠ&#280;â&#733;?H&#336;×ÉR&#258;ý\222=ë´óÚ&#318;&#366;&#323;ÁŠ\"&#711;@\203=&#350;ú]&#317;Í&#268;XÇSVôÔ\205Ž\207¨é\221."...,

  reserved = "\001\006\000\001\f&\002\000\220E&&#283;?#&#317;\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&#345;\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

Reply via email to