php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #81518 PHP header injection via default_mimetype / default_charset ini settings
Submitted: 2021-10-10 20:35 UTC Modified: 2021-10-11 17:41 UTC
From: hanskrentel at yahoo dot de Assigned: cmb (profile)
Status: Closed Package: PHP options/info functions
PHP Version: 8.0.11 OS: Linux
Private report: No CVE-ID: None
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: hanskrentel at yahoo dot de
New email:
PHP Version: OS:

 

 [2021-10-10 20:35 UTC] hanskrentel at yahoo dot de
Description:
------------
By crafting a specifically default_mimetype ini setting, it is possible
to circumvent header() injection protection and smuggle lines into the
response.

Code Example:

<?php
ini_set("default_mimetype",
        "text/html;charset=UTF-8\r\nContent-Length: 31\r\n\r\n" .
        "Lets smuggle a HTTP response.\r\n");
phpinfo();
?>

Response:

~~~
$ curl --no-progress-meter -i 'http://[::1]:8011'
HTTP/1.1 200 OK
Host: [::1]:8011
Date: Sun, 10 Oct 2021 17:01:05 GMT
Connection: close
X-Powered-By: PHP/8.1.0RC2
Content-type: text/html;charset=UTF-8
Content-Length: 31

Lets smuggle a HTTP response.
~~~

Setting the Content-Type header later again from php userland would
override it, which can be protected by sending headers with ob_flush()
or perhaps flush().

Similar injection is possible with default_charset, with differences in
the semantics:

<?php
ini_set('default_charset', 'UTF-8' . "\r\nHeader-Injection: Works!");
header('Content-Type: text/');       // trigger header injection
ob_flush();                          // trigger sending headers
ini_set('default_charset', 'UTF-8'); // restore default_charset as otherwise
                                     // htmlspecialchars() etc. give warning
?>

Ran over this while fiddling with the built-in developmenet server, can
imagine other SAPIs are affected as well and therefore considered to
report it. But I have not looked if it is SAPI specific.

To reproduce with the built-in webserver:

1. Start with a temporary router script that has the payload, e.g. in zsh:

~~~
$ php8.1 -S '[::1]:8011' =(<<<'<?php
ini_set("default_mimetyp".($_GET["a"]??""),
        "text/html;charset=UTF-8\r\nContent-Length: 31\r\n\r\n" .
        "Lets smuggle a HTTP response.\r\n");
phpinfo();')
[Sun Oct 10 18:56:30 2021] PHP 8.1.0RC2 Development Server (http://[::1]:8011) started
~~~

2. in a different terminal request (with triggering the payload):

~~~
$ curl --no-progress-meter -i 'http://[::1]:8011?a=e'
HTTP/1.1 200 OK
Host: [::1]:8011
Date: Sun, 10 Oct 2021 17:09:24 GMT
Connection: close
X-Powered-By: PHP/8.1.0RC2
Content-type: text/html;charset=UTF-8
Content-Length: 31

Lets smuggle a HTTP response.
~~~

Normally a header() call prevents such injections as no new-lines are
allowed:

<?php
header("Foo: Bar\r\nHeader-Injection: Works not.");
// Warning: Header may not contain more than a single header, new line detected
?>

Using the Content-Type header together with the ini-settings allows to
circumvent that.

It is also possible to inject new lines with a php.ini file:

~~~
$ php8.0 -S '[::1]:8011' -c =(<<<$'default_mimetype="text/plainer\r\n on the run"') =(<<<'')
[Sun Oct 10 20:35:15 2021] PHP 8.0.11 Development Server (http://[::1]:8011) started
...
~~~

~~~
$ curl --no-progress-meter -i 'http://[::1]:8011'
HTTP/1.1 200 OK
Host: [::1]:8011
Date: Sun, 10 Oct 2021 18:36:06 GMT
Connection: close
X-Powered-By: PHP/8.0.11
Content-type: text/plainer
 on the run; charset=UTF-8
~~~

To prevent header injection, the ini settings default_mimetype and
default_charset must not contain new lines.

Similiar to how the header() function rejects new lines, these two
ini settings need too, as well, to prevent header injection, as their
values are put into the Content-Type header.


Patches

Pull Requests

Pull requests:

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2021-10-11 14:44 UTC] cmb@php.net
-Package: Unknown/Other Function +Package: PHP options/info functions -Assigned To: +Assigned To: stas
 [2021-10-11 14:44 UTC] cmb@php.net
I can confirm this issue; for (F)CGI as well.  However, it doesn't
look like a security issue to me, since an exploit would require
to set these INI settings to improperly validated/sanitized user
input (or another already existing RCE vulnerability).

Stas, what do you think?

Anyhow, a quick-fix for the default_charset case could be
something like:


 main/main.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/main/main.c b/main/main.c
index 533417a569..5438d7757d 100644
--- a/main/main.c
+++ b/main/main.c
@@ -614,6 +614,12 @@ PHPAPI void (*php_internal_encoding_changed)(void) = NULL;
  */
 static PHP_INI_MH(OnUpdateDefaultCharset)
 {
+	char *s = ZSTR_VAL(new_value), *e = s + ZSTR_LEN(new_value);
+	while (--e >= s) {
+		if (*e == '\r' || *e == '\n') {
+			return FAILURE;
+		}
+	}
 	OnUpdateString(entry, new_value, mh_arg1, mh_arg2, mh_arg3, stage);
 	if (php_internal_encoding_changed) {
 		php_internal_encoding_changed();
 [2021-10-11 17:41 UTC] stas@php.net
-Type: Security +Type: Bug -Assigned To: stas +Assigned To: cmb
 [2021-10-11 17:41 UTC] stas@php.net
I don't see how it is a security issue - if you have untrusted input in your INI settings, you may have much bigger problems than header injection. That said, sanitizing the default probably won't hurt, but I don't think it's a security issue in any form.
 [2021-10-12 12:36 UTC] cmb@php.net
The following pull request has been associated:

Patch Name: Fix #81518: Header injection via default_mimetype / default_charset
On GitHub:  https://github.com/php/php-src/pull/7574
Patch:      https://github.com/php/php-src/pull/7574.patch
 [2021-10-14 10:24 UTC] git@php.net
Automatic comment on behalf of cmb69
Revision: https://github.com/php/php-src/commit/365769366b8127aab730ce0f6eaa00e16c4dbcd1
Log: Fix #81518: Header injection via default_mimetype / default_charset
 [2021-10-14 10:24 UTC] git@php.net
-Status: Assigned +Status: Closed
 [2021-11-15 11:33 UTC] divinity76 at gmail dot com
fwiw i'm against this change, IMO supporting multiple headers in default_mimetype was more akin to a feature than a bug.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 21 17:01:32 2024 UTC