Fix #71592: External entity processing never fails

If the callback set via `xml_set_external_entity_ref_handler()` returns
a falsy value, parsing is supposed to stop and the error number set to
`XML_ERROR_EXTERNAL_ENTITY_HANDLING`.  This is already correctly done
by the libexpat binding, but the libxml2 binding ignores the return
value.  We fix this by calling `xmlStopParser()` which is available as
of libxml 2.1.0[1] (PHP-7.1 requires at least libxml 2.6.11 anyway),
and setting the desired `errNo` ourselves.

[1] <http://xmlsoft.org/news.html>

diff --git a/NEWS b/NEWS
index aa12469b6aac2610c498a3445ef2a4dfd37edc10..b0ce0d795c43bd2a9f7585357504d2e6c349fbd2 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -9,6 +9,9 @@ PHP                                                                        NEWS
   . Fixed bug #50675 (SoapClient can't handle object references correctly).
     (Cameron Porter)
 
+- XML:
+  . Fixed bug 71592 (External entity processing never fails). (cmb)
+
 25 Oct 2018, PHP 7.3.0RC4
 
 - Core:

diff --git a/UPGRADING b/UPGRADING
index 369e194cb0dee8e98fa2301022bfc1529ebe52d8..d1d9d0629c28c098bf73224a3f9412c167af61df 100644 (file)
--- a/UPGRADING
+++ b/UPGRADING
@@ -482,6 +482,11 @@ PCRE:
     supported transparently. Since tidyp offers no API to get the release date,
     tidy_get_release() and tidy::getRelease() return 'unknown' in this case.
 
+ XML:
+  . The return value of the `xml_set_external_entity_ref_handler()` callback is
+    now also heeded if the extension has been built against libxml. Formerly,
+    the return value has been ignored, and parsing did never stop.
+
  Zip:
   . Building against the bundled libzip is discouraged, but still possible by
     adding `--without-libzip` to the configuration.

diff --git a/ext/xml/compat.c b/ext/xml/compat.c
index 2018dfa126aee78e5698d6dc7bac72b2989879c9..450bb1b52cc7c71064b20b3948bdd8f98af8fa15 100644 (file)
--- a/ext/xml/compat.c
+++ b/ext/xml/compat.c
@@ -359,7 +359,10 @@ _external_entity_ref_handler(void *user, const xmlChar *names, int type, const x
                return;
        }
 
-       parser->h_external_entity_ref(parser, names, (XML_Char *) "", sys_id, pub_id);
+       if (!parser->h_external_entity_ref(parser, names, (XML_Char *) "", sys_id, pub_id)) {
+               xmlStopParser(parser->parser);
+               parser->parser->errNo = XML_ERROR_EXTERNAL_ENTITY_HANDLING;
+       };
 }
 
 static xmlEntityPtr

diff --git a/ext/xml/tests/bug71592.phpt b/ext/xml/tests/bug71592.phpt
new file mode 100644 (file)
index 0000000..28a316a
--- /dev/null
+++ b/ext/xml/tests/bug71592.phpt
@@ -0,0 +1,30 @@
+--TEST--
+Bug #71592 (External entity processing never fails)
+--SKIPIF--
+<?php
+if (!extension_loaded('xml')) die('skip xml extension not available');
+?>
+--FILE--
+<?php
+$xml = <<<XML
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE p [
+  <!ENTITY pic PUBLIC "image.gif" "http://example.org/image.gif">
+]>
+<root>
+<p>&pic;</p>
+<p></nop>
+</root>
+XML;
+
+$parser = xml_parser_create_ns('UTF-8');
+xml_set_external_entity_ref_handler($parser, function () {
+    return false;
+});
+xml_parse($parser, $xml);
+var_dump(xml_get_error_code($parser));
+?>
+===DONE===
+--EXPECT--
+int(21)
+===DONE===