php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Sec Bug #76249 stream filter convert.iconv leads to infinite loop on invalid sequence
Submitted: 2018-04-22 15:18 UTC Modified: 2018-04-29 20:47 UTC
From: p dot mehrer at metaways dot de Assigned: stas (profile)
Status: Closed Package: Streams related
PHP Version: 7.0.29 OS: ubuntu/xenial x64
Private report: No CVE-ID: 2018-10546
 [2018-04-22 15:18 UTC] p dot mehrer at metaways dot de
Description:
------------
using a stream filter with convert.iconv and not enough input bytes leads to an infinite loop and one CPU Core at 100%

the test script should reproduce it very well. It is reproducable using the xenial packages as well as with a self compiled version (7.0.29) for better gdb trace.

Note: it only happens using a stream filter, php iconv() does not produce the issue.

0x00007fbaa84fc844 in __gconv (cd=0x1d30fc0, inbuf=inbuf@entry=0x7ffe4a741c30, inbufend=0x7fbaa5a71493 "",
    outbuf=outbuf@entry=0x7ffe4a741c28, outbufend=<optimized out>, irreversible=irreversible@entry=0x7ffe4a741b98) at gconv.c:79
79    gconv.c: No such file or directory.
(gdb) bt
#0  0x00007fbaa84fc844 in __gconv (cd=0x1d30fc0, inbuf=inbuf@entry=0x7ffe4a741c30, inbufend=0x7fbaa5a71493 "",
    outbuf=outbuf@entry=0x7ffe4a741c28, outbufend=<optimized out>, irreversible=irreversible@entry=0x7ffe4a741b98) at gconv.c:79
#1  0x00007fbaa84fc08f in iconv (cd=<optimized out>, inbuf=0x7ffe4a741c30, inbytesleft=0x7ffe4a741c48, outbuf=0x7ffe4a741c28,
    outbytesleft=0x7ffe4a741c38) at iconv.c:52
#2  0x0000000000664aa8 in php_iconv_stream_filter_append_bucket (self=0x7fbaa5a71460, stream=0x7fbaa5a5ea00, filter=0x7fbaa5a68150,
    buckets_out=0x7ffe4a741d80, ps=0x0, buf_len=0, consumed=0x7ffe4a741cf0, persistent=0)
    at /home/osboxes/php7.0/php-src/ext/iconv/iconv.c:2629
#3  0x0000000000665423 in php_iconv_stream_filter_do_filter (stream=0x7fbaa5a5ea00, filter=0x7fbaa5a68150, buckets_in=0x7ffe4a741d90,
    buckets_out=0x7ffe4a741d80, bytes_consumed=0x0, flags=2) at /home/osboxes/php7.0/php-src/ext/iconv/iconv.c:2828
#4  0x000000000081092c in _php_stream_fill_read_buffer (stream=0x7fbaa5a5ea00, size=8195)
    at /home/osboxes/php7.0/php-src/main/streams/streams.c:593
#5  0x0000000000810fb7 in _php_stream_read (stream=0x7fbaa5a5ea00, buf=0x7fbaa5a79018 "\205", size=8195)
    at /home/osboxes/php7.0/php-src/main/streams/streams.c:722
#6  0x0000000000812ba0 in _php_stream_copy_to_mem (src=0x7fbaa5a5ea00, maxlen=0, persistent=0, __php_stream_call_depth=0,
    __zend_filename=0xd96d78 "/home/osboxes/php7.0/php-src/ext/standard/streamsfuncs.c", __zend_lineno=443, __zend_orig_filename=0x0,
    __zend_orig_lineno=0) at /home/osboxes/php7.0/php-src/main/streams/streams.c:1473
#7  0x00000000007d4496 in zif_stream_get_contents (execute_data=0x7fbaa5a14100, return_value=0x7fbaa5a140f0)
    at /home/osboxes/php7.0/php-src/ext/standard/streamsfuncs.c:443
#8  0x00000000008ea263 in ZEND_DO_ICALL_SPEC_HANDLER () at /home/osboxes/php7.0/php-src/Zend/zend_vm_execute.h:586
#9  0x00000000008e9c8f in execute_ex (ex=0x7fbaa5a14030) at /home/osboxes/php7.0/php-src/Zend/zend_vm_execute.h:414
#10 0x00000000008e9da0 in zend_execute (op_array=0x7fbaa5a83000, return_value=0x0)
    at /home/osboxes/php7.0/php-src/Zend/zend_vm_execute.h:458
#11 0x000000000088a32a in zend_execute_scripts (type=8, retval=0x0, file_count=3) at /home/osboxes/php7.0/php-src/Zend/zend.c:1445
#12 0x00000000007f1fd8 in php_execute_script (primary_file=0x7ffe4a744680) at /home/osboxes/php7.0/php-src/main/main.c:2516
#13 0x0000000000953517 in do_cli (argc=2, argv=0x1c312d0) at /home/osboxes/php7.0/php-src/sapi/cli/php_cli.c:977
#14 0x00000000009546e5 in main (argc=2, argv=0x1c312d0) at /home/osboxes/php7.0/php-src/sapi/cli/php_cli.c:1347

Test script:
---------------
<?php
$fh = fopen('php://memory', 'rw');
fwrite($fh, "abc");
rewind($fh);
stream_filter_append($fh, 'convert.iconv.iso-10646/utf8//IGNORE', STREAM_FILTER_READ, []);
echo stream_get_contents($fh);


Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2018-04-22 16:11 UTC] p dot mehrer at metaways dot de
this patch seems to fix the issue:

diff --git a/ext/iconv/iconv.c b/ext/iconv/iconv.c
index 47aa983..789a4ce 100644
--- a/ext/iconv/iconv.c
+++ b/ext/iconv/iconv.c
@@ -2648,6 +2648,8 @@ static int php_iconv_stream_filter_append_bucket(
                                                                tcnt = 0;
                                                                break;
                                                        }
+                                               } else {
+                                                       tcnt = 0;
                                                }
                                                break;
 [2018-04-22 16:40 UTC] p dot mehrer at metaways dot de
the test script reproduced the behavior on ubuntu xenial x64 with:
7.1.16-1+ubuntu16.04.1+deb.sury.org+1
7.2.4-1+ubuntu16.04.1+deb.sury.org+1
 [2018-04-23 01:30 UTC] p dot mehrer at metaways dot de
after some meditation I have to change the description of the bug to:

a stream filter with convert.iconv will end up in an endless loop if the last bytes in the stream (before EOF) are a illegal character sequence.

the patch to fix this is:

diff --git a/ext/iconv/iconv.c b/ext/iconv/iconv.c
index 47aa983..3d0ce76 100644
--- a/ext/iconv/iconv.c
+++ b/ext/iconv/iconv.c
@@ -2648,6 +2648,9 @@ static int php_iconv_stream_filter_append_bucket(
                                                                tcnt = 0;
                                                                break;
                                                        }
+                                               } else {
+                                                       php_error_docref(NULL, E_WARNING, "iconv stream filter (\"%s\"=>\"%s\"): invalid multibyte sequence", self->from_charset, self->to_charset);
+                                                       goto out_failure;
                                                }
                                                break;


also the //ignore part (_php_check_ignore) is not taking into consideration in the stream filter. That's an other bug to me.
 [2018-04-23 03:50 UTC] stas@php.net
-Status: Open +Status: Assigned -Assigned To: +Assigned To: stas
 [2018-04-23 03:50 UTC] stas@php.net
Can reproduce it. Looks like this is a security issue since char conversions are applied to user data and function call itself seems to be completely mundane.
 [2018-04-23 03:50 UTC] stas@php.net
-Summary: DOS: stream filter convert.iconv leads to infinite loop (CPU@100%) +Summary: stream filter convert.iconv leads to infinite loop on invalid sequence
 [2018-04-23 04:28 UTC] stas@php.net
-CVE-ID: +CVE-ID: needed
 [2018-04-23 04:28 UTC] stas@php.net
Fix in security repo as 06d309fd7a917575d65c7a6f4f57b0e6bb0f9711 and in https://gist.github.com/smalyshev/02d183a38a12038fff8d65ac82d46349. Please verify.
 [2018-04-24 05:13 UTC] stas@php.net
-Status: Assigned +Status: Closed
 [2018-04-24 05:13 UTC] stas@php.net
The fix for this bug has been committed.

Snapshots of the sources are packaged every three hours; this change
will be in the next snapshot. You can grab the snapshot at
http://snaps.php.net/.

 For Windows:

http://windows.php.net/snapshots/
 
Thank you for the report, and for helping us make PHP better.


 [2018-04-29 20:47 UTC] kaplan@php.net
-CVE-ID: needed +CVE-ID: 2018-10546
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Mar 28 17:01:29 2024 UTC