php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #61807 Buffer Overflow in apache_request_headers
Submitted: 2012-04-22 01:12 UTC Modified: 2013-02-23 11:44 UTC
From: nyt-php at countercultured dot net Assigned: stas
Status: Closed Package: CGI/CLI related
PHP Version: 5.4.1RC2 OS: any
Private report: No CVE-ID: 2012-2329
 [2012-04-22 01:12 UTC] nyt-php at countercultured dot net
Description:
------------
Resubmitting since the patch didn't include last time, and I cannot view or edit my own security bug.  Makes sense :(


apache_request_headers has a loop starting at line 1617 which copies the name of an environment variable while fixing case and converting '_' to '-'.  This loop is only supposed to copy the variable name (eg: Cookie from HTTP_COOKIE), however it continues until the end of the entire environment variable.  The problem is it is copying it into buffer t, which is a pointer to buf[128].  If the string being copied is longer than 128 bytes, it will overflow the buffer.

This seems to be present in 5.4.x


Patch in case it doesn't include again:

diff --git a/sapi/cgi/cgi_main.c b/sapi/cgi/cgi_main.c
index 4643882..ef876fb 100644
--- a/sapi/cgi/cgi_main.c
+++ b/sapi/cgi/cgi_main.c
@@ -1614,7 +1614,9 @@ PHP_FUNCTION(apache_request_headers) /* {{{ */
                                var = q = t;
                                *q++ = *p++;
                                while (*p) {
-                                       if (*p == '_') {
+                                       if (*p == '=') {
+                                               break;
+                                       } else if (*p == '_') {
                                                *q++ = '-';
                                                p++;
                                                if (*p) {



Test script:
---------------
Running roundcube with php 5.4 will trigger this bug due to the large amount of cookie data and other stack conditions.  This is where I discovered it.  The included patch fixes this bug.

Expected result:
----------------
PHP should function normally

Actual result:
--------------
PHP seg faults in zif_apache_request_headers 


#5  0x000002dac7160040 in __stack_chk_fail () at stack_chk_fail.c:29
No locals.
#6  0x000000000074db0e in zif_apache_request_headers (ht=0, return_value=0x2dac596e180,
    return_value_ptr=0x3802d971bf0, this_ptr=0x6f72203b33396636,
    return_value_used=1685382481) at /tmp/buildd/php5-5.4.1~rc1/sapi/cgi/cgi_main.c:1647
        buf = "\023\000\000\000\000\000\000\000X-Roundcube-Request=4613f5de1ef497b2c16f60deb9be83de\000\060) gecko/20100101 firefox/11.0\000sviewsplitter=296; identviewsplitter"
        env = 0x3802d974638
        q = 0x3802d971bf0 "undcube-sessauth=sdcc36a0f8bd6c79753726fdf6b6818a276a2581a"
        var = 0x6f72203b33396636 <Address 0x6f72203b33396636 out of bounds>
        alloc_size = 128
        buf = "\023\000\000\000\000\000\000\000X-Roundcube-Request=4613f5de1ef497b2c16f60deb9be83de\000\060) gecko/20100101 firefox/11.0\000sviewsplitter=296; identviewsplitter"
        buf = "\023\000\000\000\000\000\000\000X-Roundcube-Request=4613f5de1ef497b2c16f60deb9be83de\000\060) gecko/20100101 firefox/11.0\000sviewsplitter=296; identviewsplitter"
#7  0x6f72203b33396636 in ?? ()
No symbol table info available.
#8  0x2d65627563646e75 in ?? ()
No symbol table info available.

Patches

apache_request_headers.phpt (last revision 2012-05-05 06:20 UTC) by remi@php.net)
apache_request_headers.patch (last revision 2012-05-05 06:19 UTC) by remi@php.net)
php-apache-request-headers.patch (last revision 2012-04-22 01:12 UTC) by nyt-php at countercultured dot net)

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2012-04-22 01:13 UTC] nyt-php at countercultured dot net
This is a security bug, but could not attach a patch to security bug, so changing a regular bug to security.
 [2012-04-22 01:13 UTC] nyt-php at countercultured dot net
-Type: Bug +Type: Security -Private report: No +Private report: Yes
 [2012-05-05 06:19 UTC] remi@php.net
The following patch has been added/updated:

Patch Name: apache_request_headers.patch
Revision:   1336198767
URL:        https://bugs.php.net/patch-display.php?bug=61807&patch=apache_request_headers.patch&revision=1336198767
 [2012-05-05 06:20 UTC] remi@php.net
The following patch has been added/updated:

Patch Name: apache_request_headers.phpt
Revision:   1336198824
URL:        https://bugs.php.net/patch-display.php?bug=61807&patch=apache_request_headers.phpt&revision=1336198824
 [2012-05-05 06:23 UTC] remi@php.net
First patch doesn't fix case where the name end by a _
Here is a test case for this issue (sapi/cgi/tests/012.phpt)

$ TEST_PHP_EXECUTABLE=/usr/bin/php pear run-tests 012.phpt
Running 1 tests
*** stack smashing detected ***: /usr/bin/php-cgi terminated
*** stack smashing detected ***: /usr/bin/php-cgi terminated
*** stack smashing detected ***: /usr/bin/php-cgi terminated
FAIL apache_request_headers() stack overflow.[012.phpt]
wrote log to "/dev/shm/php-5.4.2/sapi/cgi/tests/run-tests.log"
TOTAL TIME: 00:01
0 PASSED TESTS
0 SKIPPED TESTS
1 FAILED TESTS:
012.phpt

With the improved patch

$ TEST_PHP_EXECUTABLE=/usr/bin/php pear run-tests 012.phpt
Running 1 tests
PASS apache_request_headers() stack overflow.[012.phpt]
TOTAL TIME: 00:01
1 PASSED TESTS
0 SKIPPED TESTS
 [2012-05-20 17:51 UTC] felipe@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: stas -CVE-ID: +CVE-ID: 2012-2329
 [2012-05-20 19:50 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2012-05-20 19:50 UTC] stas@php.net
Fix checked in for 5.4.3, please tell if it doesn't work for you.
 
PHP Copyright © 2001-2014 The PHP Group
All rights reserved.
Last updated: Wed Apr 23 07:02:14 2014 UTC