php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #73246 XMLReader: encoding length not checked
Submitted: 2016-10-04 18:00 UTC Modified: 2021-04-22 09:47 UTC
From: fernando at null-life dot com Assigned: cmb (profile)
Status: Closed Package: XML Reader
PHP Version: 7.0.11 OS: Linux x64
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 this is not your bug, you can add a comment by following this link.
If this is your bug, but you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: fernando at null-life dot com
New email:
PHP Version: OS:

 

 [2016-10-04 18:00 UTC] fernando at null-life dot com
Description:
------------
When "encoding" length parameter is equal or close to 0x7fffff, stack-overflow happens in libc.so. 

Some affected functions are: XMLReader::open and XMLReader::xml

-----------------------------------
Source code:
https://github.com/php/php-src/blob/master/ext/xmlreader/php_xmlreader.c#L851

PHP_METHOD(xmlreader, open)
{
	zval *id;
	size_t source_len = 0, encoding_len = 0;
	zend_long options = 0;
	xmlreader_object *intern = NULL;
	char *source, *valid_file = NULL;
	char *encoding = NULL;
	char resolved_path[MAXPATHLEN + 1];
	xmlTextReaderPtr reader = NULL;

	if (zend_parse_parameters(ZEND_NUM_ARGS(), "p|s!l", &source, &source_len, &encoding, &encoding_len, &options) == FAILURE) {
		return;
	}

	id = getThis();
	if (id != NULL) {
		if (! instanceof_function(Z_OBJCE_P(id), xmlreader_class_entry)) {
			id = NULL;
		} else {
			intern = Z_XMLREADER_P(id);
			xmlreader_free_resources(intern);
		}
	}

	if (!source_len) {
		php_error_docref(NULL, E_WARNING, "Empty string supplied as input");
		RETURN_FALSE;
	}

	valid_file = _xmlreader_get_valid_file_path(source, resolved_path, MAXPATHLEN );

	if (valid_file) {
		reader = xmlReaderForFile(valid_file, encoding, options);  // encoding length not checked
	}
...

https://github.com/php/php-src/blob/master/ext/xmlreader/php_xmlreader.c#L1035
PHP_METHOD(xmlreader, XML)
{
	zval *id;
	size_t source_len = 0, encoding_len = 0;
	zend_long options = 0;
	xmlreader_object *intern = NULL;
	char *source, *uri = NULL, *encoding = NULL;
	int resolved_path_len, ret = 0;
	char *directory=NULL, resolved_path[MAXPATHLEN];
	xmlParserInputBufferPtr inputbfr;
	xmlTextReaderPtr reader;

	if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|s!l", &source, &source_len, &encoding, &encoding_len, &options) == FAILURE) {
		return;
	}

	id = getThis();
	if (id != NULL && ! instanceof_function(Z_OBJCE_P(id), xmlreader_class_entry)) {
		id = NULL;
	}
	if (id != NULL) {
		intern = Z_XMLREADER_P(id);
		xmlreader_free_resources(intern);
	}

	if (!source_len) {
		php_error_docref(NULL, E_WARNING, "Empty string supplied as input");
		RETURN_FALSE;
	}

	inputbfr = xmlParserInputBufferCreateMem(source, source_len, XML_CHAR_ENCODING_NONE);

    if (inputbfr != NULL) {
/* Get the URI of the current script so that we can set the base directory in libxml */
#if HAVE_GETCWD
		directory = VCWD_GETCWD(resolved_path, MAXPATHLEN);
#elif HAVE_GETWD
		directory = VCWD_GETWD(resolved_path);
#endif
		if (directory) {
			resolved_path_len = strlen(resolved_path);
			if (resolved_path[resolved_path_len - 1] != DEFAULT_SLASH) {
				resolved_path[resolved_path_len] = DEFAULT_SLASH;
				resolved_path[++resolved_path_len] = '\0';
			}
			uri = (char *) xmlCanonicPath((const xmlChar *) resolved_path);
		}
		reader = xmlNewTextReader(inputbfr, uri);

		if (reader != NULL) {
#if LIBXML_VERSION >= 20628
			ret = xmlTextReaderSetup(reader, NULL, uri, encoding, options); // encoding length not checked
#endif
...



GDB output:

USE_ZEND_ALLOC=0 ASAN_OPTIONS=detect_leaks=0 gdb -q --args /home/operac/build4/bin/php -n poc.php
No symbol table is loaded.  Use the "file" command.
Breakpoint 1 (__asan_report_error) pending.
Reading symbols from /home/operac/build4/bin/php...done.
gdb-peda$ b xmlReaderForFile
Breakpoint 2 at 0x42e530
gdb-peda$ r
Starting program: /home/operac/build4/bin/php -n poc.php
...
Breakpoint 2, 0x00007ffff5773f90 in xmlReaderForFile () from /usr/lib/x86_64-linux-gnu/libxml2.so.2
gdb-peda$ p strlen($rsi)
$2 = 0x7ffffe
...




Test script:
---------------
poc.php:

<?php

ini_set('memory_limit', -1);

$v = str_repeat("/", 0xffffff/2 - 1);
XMLReader::open(".", $v);


poc2.php:
<?php

ini_set('memory_limit', -1);

$v = str_repeat("/", 0xffffff/2);
XMLReader::xml("<xml>", $v);

Expected result:
----------------
No crash

Actual result:
--------------

ASan output:

USE_ZEND_ALLOC=0 ASAN_OPTIONS=detect_leaks=0 /home/operac/build4/bin/php  -n poc.php
ASAN:SIGSEGV
=================================================================
==7571==ERROR: AddressSanitizer: stack-overflow on address 0x7ffd866baf28 (pc 0x7f684a41b4f9 bp 0x7ffd86ebafd0 sp 0x7ffd866baf30 T0)
    #0 0x7f684a41b4f8  (/lib/x86_64-linux-gnu/libc.so.6+0x214f8)
    #1 0x7f684a41ae29 in iconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x20e29)
    #2 0x7f684b14c908 in xmlFindCharEncodingHandler (/usr/lib/x86_64-linux-gnu/libxml2.so.2+0x32908)
    #3 0x7f684b23f1e6 in xmlReaderForFile (/usr/lib/x86_64-linux-gnu/libxml2.so.2+0x1251e6)
    #4 0x1668f3a in zim_xmlreader_open /home/operac/build4/php-src/ext/xmlreader/php_xmlreader.c:884
    #5 0x1d74293 in ZEND_DO_FCALL_SPEC_HANDLER /home/operac/build4/php-src/Zend/zend_vm_execute.h:842
    #6 0x1b9ff15 in execute_ex /home/operac/build4/php-src/Zend/zend_vm_execute.h:414
    #7 0x1e4e7a8 in zend_execute /home/operac/build4/php-src/Zend/zend_vm_execute.h:458
    #8 0x199ce7c in zend_execute_scripts /home/operac/build4/php-src/Zend/zend.c:1427
    #9 0x170fda7 in php_execute_script /home/operac/build4/php-src/main/main.c:2494
    #10 0x1e56a32 in do_cli /home/operac/build4/php-src/sapi/cli/php_cli.c:974
    #11 0x46e424 in main /home/operac/build4/php-src/sapi/cli/php_cli.c:1344
    #12 0x7f684a41a82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #13 0x46eaf8 in _start (/home/operac/build4/bin/php+0x46eaf8)

SUMMARY: AddressSanitizer: stack-overflow ??:0 ??
==7571==ABORTING


USE_ZEND_ALLOC=0 ASAN_OPTIONS=detect_leaks=0 /home/operac/build4/bin/php  -n poc2.php
ASAN:SIGSEGV
=================================================================
==7572==ERROR: AddressSanitizer: stack-overflow on address 0x7ffee82e1f78 (pc 0x7fd94762c4f9 bp 0x7ffee8ae2020 sp 0x7ffee82e1f80 T0)
    #0 0x7fd94762c4f8  (/lib/x86_64-linux-gnu/libc.so.6+0x214f8)
    #1 0x7fd94762be29 in iconv_open (/lib/x86_64-linux-gnu/libc.so.6+0x20e29)
    #2 0x7fd94835d908 in xmlFindCharEncodingHandler (/usr/lib/x86_64-linux-gnu/libxml2.so.2+0x32908)
    #3 0x7fd94844f795 in xmlTextReaderSetup (/usr/lib/x86_64-linux-gnu/libxml2.so.2+0x124795)
    #4 0x166455d in zim_xmlreader_XML /home/operac/build4/php-src/ext/xmlreader/php_xmlreader.c:1086
    #5 0x1d74293 in ZEND_DO_FCALL_SPEC_HANDLER /home/operac/build4/php-src/Zend/zend_vm_execute.h:842
    #6 0x1b9ff15 in execute_ex /home/operac/build4/php-src/Zend/zend_vm_execute.h:414
    #7 0x1e4e7a8 in zend_execute /home/operac/build4/php-src/Zend/zend_vm_execute.h:458
    #8 0x199ce7c in zend_execute_scripts /home/operac/build4/php-src/Zend/zend.c:1427
    #9 0x170fda7 in php_execute_script /home/operac/build4/php-src/main/main.c:2494
    #10 0x1e56a32 in do_cli /home/operac/build4/php-src/sapi/cli/php_cli.c:974
    #11 0x46e424 in main /home/operac/build4/php-src/sapi/cli/php_cli.c:1344
    #12 0x7fd94762b82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #13 0x46eaf8 in _start (/home/operac/build4/bin/php+0x46eaf8)

SUMMARY: AddressSanitizer: stack-overflow ??:0 ??

Patches

Add a Patch

Pull Requests

Pull requests:

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2016-11-04 01:06 UTC] stas@php.net
-Status: Open +Status: Feedback
 [2016-11-04 01:06 UTC] stas@php.net
Failed to reproduce any problem on my system. Maybe a bug in that particular libc build. Does it reproduce on standard PHP build?
 [2016-11-06 19:03 UTC] fernando at null-life dot com
-Status: Feedback +Status: Open
 [2016-11-06 19:03 UTC] fernando at null-life dot com
I'm testing on a updated Ubuntu x86_64 Xenial (16.04.1), it's not the last PHP release but AFAIK there has been no changes related to XMLReader to recent PHP versions:

Full data environment

---- PHP --------------------------------------------

php --version
PHP 7.0.8-0ubuntu0.16.04.3 (cli) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.8-0ubuntu0.16.04.3, Copyright (c) 1999-2016, by Zend Technologies

---- PHP XML ---------------------------------------

Package: php7.0-xml
Priority: optional
Section: php
Installed-Size: 468
Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
Original-Maintainer: Debian PHP Maintainers <pkg-php-maint@lists.alioth.debian.org>
Architecture: amd64
Source: php7.0
Version: 7.0.8-0ubuntu0.16.04.3


---- libxml2 ----------------------------------------

Package: libxml2
Priority: standard
Section: libs
Installed-Size: 2124
Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
Original-Maintainer: Debian XML/SGML Group <debian-xml-sgml-pkgs@lists.alioth.debian.org>
Architecture: amd64
Version: 2.9.3+dfsg1-1ubuntu0.1

---- libc ------------------------------------

Package: libc6
Priority: required
Section: libs
Installed-Size: 10948
Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
Original-Maintainer: GNU Libc Maintainers <debian-glibc@lists.debian.org>
Architecture: amd64
Source: glibc
Version: 2.23-0ubuntu4


---- OS --------------------------------------

cat /etc/os-release
NAME="Ubuntu"
VERSION="16.04.1 LTS (Xenial Xerus)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 16.04.1 LTS"
VERSION_ID="16.04"
HOME_URL="http://www.ubuntu.com/"
SUPPORT_URL="http://help.ubuntu.com/"
BUG_REPORT_URL="http://bugs.launchpad.net/ubuntu/"
VERSION_CODENAME=xenial
UBUNTU_CODENAME=xenial

---- Arch ---------------------------------------

$ uname -a

Linux hp3 4.4.0-21-generic #37-Ubuntu SMP Mon Apr 18 18:33:37 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux

---- PoC ---------------------------------------------

$ cat poc.php
<?php

ini_set('memory_limit', -1);

$v = str_repeat("/", 0xffffff/2 - 1);
XMLReader::open(".", $v);


$ php poc.php
Segmentation fault (core dumped)
 [2017-10-17 14:40 UTC] cmb@php.net
iconv_open() is supposed to accept two const char * without any particular
restrictions on their length. So, this looks like a bug in the iconv_open()
implementation.
 [2018-09-14 12:20 UTC] cmb@php.net
-Package: *XML functions +Package: XML Reader
 [2020-03-18 12:58 UTC] cmb@php.net
-Assigned To: +Assigned To: stas
 [2020-03-18 12:58 UTC] cmb@php.net
The reported stack overflow is certainly not a PHP bug.

> encoding length not checked

This is a bug, but I'm not sure whether it is a security issue.
stas, what do you think?

Anyway, fix would be as simple as:


 ext/xmlreader/php_xmlreader.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ext/xmlreader/php_xmlreader.c b/ext/xmlreader/php_xmlreader.c
index 4d4e7348c9..f920d04eb7 100644
--- a/ext/xmlreader/php_xmlreader.c
+++ b/ext/xmlreader/php_xmlreader.c
@@ -848,7 +848,7 @@ PHP_METHOD(xmlreader, open)
 	char resolved_path[MAXPATHLEN + 1];
 	xmlTextReaderPtr reader = NULL;
 
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "p|s!l", &source, &source_len, &encoding, &encoding_len, &options) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "p|p!l", &source, &source_len, &encoding, &encoding_len, &options) == FAILURE) {
 		return;
 	}
 
@@ -1033,7 +1033,7 @@ PHP_METHOD(xmlreader, XML)
 	xmlParserInputBufferPtr inputbfr;
 	xmlTextReaderPtr reader;
 
-	if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|s!l", &source, &source_len, &encoding, &encoding_len, &options) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|p!l", &source, &source_len, &encoding, &encoding_len, &options) == FAILURE) {
 		return;
 	}
 [2021-04-21 13:27 UTC] cmb@php.net
More appropriate patch for PHP-7.4:
<https://gist.github.com/cmb69/6f8720154c016bfceeee72c400870b48>.

IMO, not a security issue.
 [2021-04-21 18:16 UTC] stas@php.net
-Type: Security +Type: Bug
 [2021-04-21 18:16 UTC] stas@php.net
The null thing doesn't look like security issue. The other thing doesn't look like PHP issue. Let's fix the null thing in 7.4+.
 [2021-04-22 09:47 UTC] cmb@php.net
-Summary: Stack overflow through XMLReader functions +Summary: XMLReader: encoding length not checked -Assigned To: stas +Assigned To: cmb
 [2021-04-22 09:49 UTC] cmb@php.net
The following pull request has been associated:

Patch Name: Fix #73246: XMLReader: encoding length not checked
On GitHub:  https://github.com/php/php-src/pull/6899
Patch:      https://github.com/php/php-src/pull/6899.patch
 [2021-05-03 10:31 UTC] git@php.net
Automatic comment on behalf of cmb69
Revision: https://github.com/php/php-src/commit/272df442f5a9617bf19e55ba5c6e3180315a18cf
Log: Fix #73246: XMLReader: encoding length not checked
 [2021-05-03 10:31 UTC] git@php.net
-Status: Assigned +Status: Closed
 
PHP Copyright © 2001-2021 The PHP Group
All rights reserved.
Last updated: Wed Jun 16 02:01:24 2021 UTC