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 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

Pull Requests

Pull requests:

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-2024 The PHP Group
All rights reserved.
Last updated: Sat Sep 21 00:01:27 2024 UTC