php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #66547 XML_OPTION_SKIP_TAGSTART information leak
Submitted: 2014-01-22 16:56 UTC Modified: 2020-12-23 15:11 UTC
Votes:1
Avg. Score:4.0 ± 0.0
Reproduced:1 of 1 (100.0%)
Same Version:0 (0.0%)
Same OS:1 (100.0%)
From: sean at persistencelabs dot com Assigned: cmb (profile)
Status: Duplicate Package: DOM XML related
PHP Version: master-Git-2014-01-22 (Git) OS:
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: sean at persistencelabs dot com
New email:
PHP Version: OS:

 

 [2014-01-22 16:56 UTC] sean at persistencelabs dot com
Description:
------------
Summary
-------
 
The xml_parser_set_option function allows one to specify a number of bytes to
skip beyond the start of a tag, before passing this tag to various
user-specified handlers. The value is not sanitised in any way and thus can be
manipulated to read data at arbitrary offsets from the tag.

Impact
------

The impact of this bug is an information leak. A malicious user can read data
at arbitrary offsets from an allocated heap chunk. i.e. it's useful for 
defeating ASLR.

Patch Details
-------------

The patch (in a comment below) updates each location that makes use of the
user-specified offset to ensure that it does not result in an out of bounds pointer.

Bug Details
-----------

The trigger file demonstrates this issue by specifying -4 as 
the offset. As shown in the attached GDB session, 
zif_xml_parser_set_option will store this value in the toffset attribute of
the xml_parser object (line 1634).

File : ext/xml/php_xml.h

46 typedef struct {
	...

81         int toffset;
	...

90 } xml_parser;

File : ext/xml/xml.c

1616 PHP_FUNCTION(xml_parser_set_option)
1617 {
1618         xml_parser *parser;
1619         zval *pind, **val;
1620         long opt;
1621 
1622         if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rlZ", &pind, &opt, &val) == FAILURE) {
1623                 return;
1624         }
1625         ZEND_FETCH_RESOURCE(parser,xml_parser *, &pind, -1, "XML Parser", le_xml_parser);
1626 
1627         switch (opt) {
1628                 case PHP_XML_OPTION_CASE_FOLDING:
1629                         convert_to_long_ex(val);
1630                         parser->case_folding = Z_LVAL_PP(val);
1631                         break;
1632                 case PHP_XML_OPTION_SKIP_TAGSTART:
1633                         convert_to_long_ex(val);
1634                         parser->toffset = Z_LVAL_PP(val);
1635                         break;

(gdb) p/x parser->toffset
$3 = 0xfffffffc

The toffset attribute is used in a number of locations. In gdbsession.txt we
can see it being used in the following code (line 787) to calculate the 
address of the string to pass to the start-element handler. By adding -4 we
skip backwards into memory located before the string.

File : ext/xml/xml.c

771 void _xml_startElementHandler(void *userData, const XML_Char *name, const XML_Char **attributes)
772 {
773         xml_parser *parser = (xml_parser *)userData;
774         const char **attrs = (const char **) attributes;
775         char *tag_name;
776         char *att, *val;
777         int val_len;
778         zval *retval, *args[3];
779         
780         if (parser) {
781                 parser->level++;
782                 
783                 tag_name = _xml_decode_tag(parser, name);
784         
785                 if (parser->startElementHandler) {
786                         args[0] = _xml_resource_zval(parser->index);
787                         args[1] = _xml_string_zval(((char *) tag_name) + parser->toffset);
788                         MAKE_STD_ZVAL(args[2]);
789                         array_init(args[2]); 
790                         
791                         while (attributes && *attributes) {
792                                 att = _xml_decode_tag(parser, attributes[0]);
793                                 val = xml_utf8_decode(attributes[1], strlen(attributes[1]), &val_len, parser->target_encoding);
794 
795                                 add_assoc_stringl(args[2], att, val, val_len, 0);
796 
797                                 attributes += 2;
798 
799                                 efree(att);
800                         }
801 
802                         if ((retval = xml_call_handler(parser, parser->startElementHandler, parser->startElementPtr, 3, args))) {
803                                 zval_ptr_dtor(&retval);
804                         }
805                 }

Breakpoint 3, _xml_startElementHandler (userData=0xb7bdb12c, name=0x8a6b510 "FOO:BAR", attributes=0x0) at ext/xml/xml.c:787
787                             args[1] = _xml_string_zval(((char *) tag_name) + parser->toffset);
(gdb) x/s tag_name
0xb7bdc0e0:      "FOO:BAR"
(gdb) p/x parser->toffset
$4 = 0xfffffffc
(gdb) s
_xml_string_zval (str=0xb7bdc0dc "\b\001") at ext/xml/xml.c:394
394             int len = strlen(str);
(gdb) x/16x str
0xb7bdc0dc:     0x08    0x01    0x00    0x00    0x46    0x4f    0x4f    0x3a
0xb7bdc0e4:     0x42    0x41    0x52    0x00    0x25    0x00    0x00    0x00
(gdb) p/x len
$5 = 0x2

As shown, the string to be passed to the element handler is now considered
to be (0x08 0x01) rather than (0x46 0x4f ...) [FOO:BAR].

The element handler then prints these byte values.

EOF

Test script:
---------------
<?php

$XML = <<<XML
<?xml version="1.0"?>
<FOO:BAR>
</FOO:BAR>
XML;

function startElement($parser, $name, $attribs) { echo bin2hex($name) . PHP_EOL; }
function endElement($parser, $name) { echo bin2hex($name) . PHP_EOL; }
$xml_parser = xml_parser_create();
xml_set_element_handler($xml_parser, 'startElement', 'endElement');
xml_parser_set_option($xml_parser, XML_OPTION_SKIP_TAGSTART, 0xfffffffc);
xml_parse($xml_parser, $XML);
xml_parser_free($xml_parser);

?>


Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2014-01-22 16:57 UTC] sean at persistencelabs dot com
(gdb) r trigger.php 
The program being debugged has been started already.
Start it from the beginning? (y or n) y

Starting program: /home/user/php/sapi/cli/php /home/user/trigger.php
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/i386-linux-gnu/libthread_db.so.1".

Breakpoint 1, zif_xml_parser_set_option (ht=3, return_value=0xb7bdb21c, return_value_ptr=0xb7bbf10c, this_ptr=0x0, return_value_used=0) at ext/xml/xml.c:1622
1622            if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rlZ", &pind, &opt, &val) == FAILURE) {
(gdb) n
1625            ZEND_FETCH_RESOURCE(parser,xml_parser *, &pind, -1, "XML Parser", le_xml_parser);
(gdb) 
1627            switch (opt) {
(gdb) 
1633                            convert_to_long_ex(val);
(gdb) 
1634                            parser->toffset = Z_LVAL_PP(val);
(gdb) 
1635                            break;
(gdb) p/x parser->toffset
$3 = 0xfffffffc
(gdb) c
Continuing.

Breakpoint 2, _xml_startElementHandler (userData=0xb7bdb12c, name=0x8a6b510 "FOO:BAR", attributes=0x0) at ext/xml/xml.c:773
773             xml_parser *parser = (xml_parser *)userData;
(gdb) l
768     /* }}} */
769
770     /* {{{ _xml_startElementHandler() */
771     void _xml_startElementHandler(void *userData, const XML_Char *name, const XML_Char **attributes)
772     {
773             xml_parser *parser = (xml_parser *)userData;
774             const char **attrs = (const char **) attributes;
775             char *tag_name;
776             char *att, *val;
777             int val_len;
(gdb) 
778             zval *retval, *args[3];
779
780             if (parser) {
781                     parser->level++;
782
783                     tag_name = _xml_decode_tag(parser, name);
784
785                     if (parser->startElementHandler) {
786                             args[0] = _xml_resource_zval(parser->index);
787                             args[1] = _xml_string_zval(((char *) tag_name) + parser->toffset);
(gdb) c
Continuing.

Breakpoint 3, _xml_startElementHandler (userData=0xb7bdb12c, name=0x8a6b510 "FOO:BAR", attributes=0x0) at ext/xml/xml.c:787
787                             args[1] = _xml_string_zval(((char *) tag_name) + parser->toffset);
(gdb) x/s tag_name
0xb7bdc0e0:      "FOO:BAR"
(gdb) p/x parser->toffset
$4 = 0xfffffffc
(gdb) s
_xml_string_zval (str=0xb7bdc0dc "\b\001") at ext/xml/xml.c:394
394             int len = strlen(str);
(gdb) x/16x str
0xb7bdc0dc:     0x08    0x01    0x00    0x00    0x46    0x4f    0x4f    0x3a
0xb7bdc0e4:     0x42    0x41    0x52    0x00    0x25    0x00    0x00    0x00
(gdb) n
395             MAKE_STD_ZVAL(ret);
(gdb) p/x len
$5 = 0x2
(gdb) c
Continuing.
 
 

0801
0801
[Inferior 1 (process 13133) exited normally]
(gdb)
 [2014-01-22 16:58 UTC] sean at persistencelabs dot com
From d6e911fa90a8cb711a9a94e6a3ae790bfe891b13 Mon Sep 17 00:00:00 2001
From: Sean Heelan <sean@persistencelabs.com>
Date: Sun, 15 Dec 2013 23:14:51 +0000
Subject: [PATCH 1/1] Check that tag + tag offset is still actually within the
 tag string

---
 ext/xml/xml.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/ext/xml/xml.c b/ext/xml/xml.c
index 1ef01c8..c2c97d0 100644
--- a/ext/xml/xml.c
+++ b/ext/xml/xml.c
@@ -781,6 +781,13 @@ void _xml_startElementHandler(void *userData, const XML_Char *name, const XML_Ch
 		parser->level++;
 
 		tag_name = _xml_decode_tag(parser, name);
+		unsigned tag_len = strlen(tag_name);
+		if (parser->toffset > tag_len) {
+			TSRMLS_FETCH();
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Tag offset > tag length");
+			return;
+		}
+
 
 		if (parser->startElementHandler) {
 			args[0] = _xml_resource_zval(parser->index);
@@ -866,6 +873,12 @@ void _xml_endElementHandler(void *userData, const XML_Char *name)
 		zval *retval, *args[2];
 
 		tag_name = _xml_decode_tag(parser, name);
+		unsigned tag_len = strlen(tag_name);
+		if (parser->toffset > tag_len) {
+			TSRMLS_FETCH();
+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "Tag offset > tag length");
+			return;
+		}
 
 		if (parser->endElementHandler) {
 			args[0] = _xml_resource_zval(parser->index);
@@ -989,6 +1002,13 @@ void _xml_characterDataHandler(void *userData, const XML_Char *s, int len)
 
 						array_init(tag);
 
+						unsigned tag_len = strlen(parser->ltags[parser->level-1]);
+						if (parser->toffset > tag_len) {
+							TSRMLS_FETCH();
+							php_error_docref(NULL TSRMLS_CC, E_WARNING, "Tag offset > tag length");
+							return;
+						}
+
 						_xml_add_to_info(parser,parser->ltags[parser->level-1] + parser->toffset);
 
 						add_assoc_string(tag,"tag",parser->ltags[parser->level-1] + parser->toffset,1);
-- 
1.7.9.5
 [2014-02-03 19:38 UTC] stas@php.net
-Type: Security +Type: Bug -Package: *General Issues +Package: DOM XML related
 [2016-01-11 12:50 UTC] sean dot heelan at gmail dot com
This issue is still present in PHP 7. Would it be possible to get the patch applied?
 [2020-12-23 15:11 UTC] cmb@php.net
-Status: Open +Status: Duplicate -Assigned To: +Assigned To: cmb
 [2020-12-23 15:11 UTC] cmb@php.net
It seems to me this has been fixed as bug #72714 in the meantime.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Dec 21 17:01:58 2024 UTC