php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #71323 Output of stream_get_meta_data can be falsified by its input
Submitted: 2016-01-10 02:44 UTC Modified: 2016-02-02 03:17 UTC
From: leo at gaspard dot io Assigned: stas (profile)
Status: Closed Package: Unknown/Other Function
PHP Version: 5.5.31 OS: Any
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: leo at gaspard dot io
New email:
PHP Version: OS:

 

 [2016-01-10 02:44 UTC] leo at gaspard dot io
Description:
------------
Hello,

Here is the text I put in the commit message of the patch I will attach to this bug report, given I don't find any better way to put it:

==========8<==========8<==========

At the moment, the return value of stream_get_meta_data is composed of
some fields that have a defined value, and some other fields that are
populated with php_stream_populate_meta_data.

This would not be a problem, if php_stream_populate_meta_data was
restricted to writing in fields that are not already set. However, it
turns out, in particular, that a php_stream with ops set to
php_stream_temp_ops fills the metadata with whatever the user supplies.

As a consequence, if we assume $file is attacker-controlled, a program
such as

    $uri = stream_get_meta_data(fopen($file, "r"))['uri'];

will yield attacker-controlled data in $uri, and not, as one would
expect, sane PHP-provided data. For instance, $uri will be set to
"eviluri" when $file is set to

    data:text/plain;uri=eviluri,

The change to ext/standard/streamsfuncs.c fixes the aforementioned issue
by first filling metadata, and only then filling fields that PHP knows
about. This way, the standardized (and documented) fields will not be
attacker-controlled.

The change made to main/streams/memory.c protects the "mediatype"
variable against the same tampering:

    data:text/plain,mediatype=text/x-php,

would set the official mediatype to text/plain, but PHP would interpret
it as text/x-php. This could confuse IDS, IPS or any other application
that reads the same data but does not use the same parser, as these
would not understand the same mediatype as the PHP application does.


==========>8==========>8==========

Hoping this helps,

Leo Gaspard

Test script:
---------------
echo stream_get_meta_data(fopen("data:text/plain;uri=eviluri,", "r"))['uri'];
echo stream_get_meta_data(fopen("data:real/evil;mediatype=text/plain,", "r"))['mediatype'];

Expected result:
----------------
data:text/plain;uri=eviluri,
real/evil

Actual result:
--------------
eviluri
text/plain

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-01-10 02:53 UTC] leo at gaspard dot io
Given it looks like the patch was not attached and I can't attach a patch now (clicking the "Add a Patch" button sends me to a "This bug-report is private"-like page -- maybe this would deserve its own bug report), here is a paste of the entire patch, as generated by git format-patch and applicable by git am -- sorry again for the inconvenience.

==========8<==========8<==========

From 1da9a5b048a31f815574a417bb45a813d69875b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?L=C3=A9o=20Gaspard?= <leo@gaspard.io>
Date: Tue, 29 Dec 2015 01:15:13 +0100
Subject: [PATCH] Mitigate return tampering in stream_get_meta_data

At the moment, the return value of stream_get_meta_data is composed of
some fields that have a defined value, and some other fields that are
populated with php_stream_populate_meta_data.

This would not be a problem, if php_stream_populate_meta_data was
restricted to writing in fields that are not already set. However, it
turns out, in particular, that a php_stream with ops set to
php_stream_temp_ops fills the metadata with whatever the user supplies.

As a consequence, if we assume $file is attacker-controlled, a program
such as

    $uri = stream_get_meta_data(fopen($file, "r"))['uri'];

will yield attacker-controlled data in $uri, and not, as one would
expect, sane PHP-provided data. For instance, $uri will be set to
"eviluri" when $file is set to

    data:text/plain;uri=eviluri,

The change to ext/standard/streamsfuncs.c fixes the aforementioned issue
by first filling metadata, and only then filling fields that PHP knows
about. This way, the standardized (and documented) fields will not be
attacker-controlled.

The change made to main/streams/memory.c protects the "mediatype"
variable against the same tampering:

    data:text/plain,mediatype=text/x-php,

would set the official mediatype to text/plain, but PHP would interpret
it as text/x-php. This could confuse IDS, IPS or any other application
that reads the same data but does not use the same parser, as these
would not understand the same mediatype as the PHP application does.
---
 ext/standard/streamsfuncs.c | 12 ++++++------
 main/streams/memory.c       |  4 +++-
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/ext/standard/streamsfuncs.c b/ext/standard/streamsfuncs.c
index de0f016..496b990 100644
--- a/ext/standard/streamsfuncs.c
+++ b/ext/standard/streamsfuncs.c
@@ -507,6 +507,12 @@ PHP_FUNCTION(stream_get_meta_data)

        array_init(return_value);

+       if (!php_stream_populate_meta_data(stream, return_value)) {
+               add_assoc_bool(return_value, "timed_out", 0);
+               add_assoc_bool(return_value, "blocked", 1);
+               add_assoc_bool(return_value, "eof", php_stream_eof(stream));
+       }
+
        if (!Z_ISUNDEF(stream->wrapperdata)) {
                Z_ADDREF_P(&stream->wrapperdata);
                add_assoc_zval(return_value, "wrapper_data", &stream->wrapperdata);
@@ -539,12 +545,6 @@ PHP_FUNCTION(stream_get_meta_data)
        if (stream->orig_path) {
                add_assoc_string(return_value, "uri", stream->orig_path);
        }
-
-       if (!php_stream_populate_meta_data(stream, return_value)) {
-               add_assoc_bool(return_value, "timed_out", 0);
-               add_assoc_bool(return_value, "blocked", 1);
-               add_assoc_bool(return_value, "eof", php_stream_eof(stream));
-       }
 }
 /* }}} */

diff --git a/main/streams/memory.c b/main/streams/memory.c
index e2695ff..e36835a 100644
--- a/main/streams/memory.c
+++ b/main/streams/memory.c
@@ -697,7 +697,9 @@ static php_stream * php_stream_url_wrap_rfc2397(php_stream_wrapper *wrapper, con
                        plen = sep - path;
                        vlen = (semi ? semi - sep : mlen - plen) - 1 /* '=' */;
                        key = estrndup(path, plen);
-                       add_assoc_stringl_ex(&meta, key, plen, sep + 1, vlen);
+                       if (memcmp(key, "mediatype", sizeof("mediatype")-1)) {
+                               add_assoc_stringl_ex(&meta, key, plen, sep + 1, vlen);
+                       }
                        efree(key);
                        plen += vlen + 1;
                        mlen -= plen;
--
2.6.4
 [2016-01-10 03:37 UTC] stas@php.net
You can also use gist.github.com (choose "Secret gist") to make private patches.
 [2016-01-17 06:12 UTC] stas@php.net
-Assigned To: +Assigned To: stas
 [2016-01-17 06:12 UTC] stas@php.net
Fixed in 6297a117d77fa3a0df2e21ca926a92c231819cd5 in security repo and https://gist.github.com/smalyshev/9609440fd5eab148623a
 [2016-01-17 06:12 UTC] stas@php.net
-PHP Version: 7.0Git-2016-01-10 (Git) +PHP Version: 5.5.31
 [2016-02-02 03:19 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=6297a117d77fa3a0df2e21ca926a92c231819cd5
Log: Fixed bug #71323 - Output of stream_get_meta_data can be falsified by its input
 [2016-02-02 03:19 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2016-02-02 03:36 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=6297a117d77fa3a0df2e21ca926a92c231819cd5
Log: Fixed bug #71323 - Output of stream_get_meta_data can be falsified by its input
 [2016-02-02 04:46 UTC] stas@php.net
Automatic comment on behalf of stas
Revision: http://git.php.net/?p=php-src.git;a=commit;h=6297a117d77fa3a0df2e21ca926a92c231819cd5
Log: Fixed bug #71323 - Output of stream_get_meta_data can be falsified by its input
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Nov 07 23:01:29 2024 UTC