php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #37496 FastCGI output buffer overrun
Submitted: 2006-05-18 13:01 UTC Modified: 2006-05-25 06:41 UTC
From: gacek at intertele dot pl Assigned: dmitry (profile)
Status: Closed Package: CGI/CLI related
PHP Version: 5CVS-2006-05-18 (snap) OS: Linux
Private report: No CVE-ID: None
View Developer Edit
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: gacek at intertele dot pl
New email:
PHP Version: OS:

 

 [2006-05-18 13:01 UTC] 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.

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2006-05-22 07:03 UTC] 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-22 09:27 UTC] dmitry@php.net 
The patch is partially commited into HEAD, PHP_5_2 and PHP_5_1. Did bug go away?
 [2006-05-24 09:12 UTC] gacek at intertele dot pl 
Bug has gone.
 [2006-05-24 12:44 UTC] 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-25 06:41 UTC] dmitry@php.net 
Thank you for your help.

Fixed in CVS HEAD, PHP_5_2 and PHP_5_1.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Sat Nov 23 09:01:28 2024 UTC