Fix libxml_disable_entity_loader()

This wasn't calling requestInit and setting the libxml handler no null.
So the first time an error came along it would reset the handler from
no-op to reading again.

This is a much better fix, we set our custom handler in requestInit and
when libxml_disable_entity_loader we store that state as a member bool
ensuring requestInit is always called to set our own handler.

If the handler isn't inserted then the behavious is as before. The only
time this could go pear shaped is say we wanted to make the default be
off. In that case we'd need a global requestInit that is always called
since there are libxml references everywhere.

Reviewed By: @jdelong

Differential Revision: D1116686

hphp/runtime/ext/ext_simplexml.cpp

@@ -1246,6 +1246,9 @@ void c_LibXMLError::t___construct() {
 ///////////////////////////////////////////////////////////////////////////////
 // libxml
 
+static xmlParserInputBufferPtr
+hphp_libxml_input_buffer(const char *URI, xmlCharEncoding enc);
+
 class xmlErrorVec : public std::vector<xmlError> {
 public:
   ~xmlErrorVec() {
@@ -1265,13 +1268,15 @@ class LibXmlErrors : public RequestEventHandler {
   virtual void requestInit() {
     m_use_error = false;
     m_errors.reset();
-    xmlParserInputBufferCreateFilenameDefault(nullptr);
+    m_entity_loader_disabled = false;
+    xmlParserInputBufferCreateFilenameDefault(hphp_libxml_input_buffer);
   }
   virtual void requestShutdown() {
     m_use_error = false;
     m_errors.reset();
   }
 
+  bool m_entity_loader_disabled;
   bool m_use_error;
   xmlErrorVec m_errors;
 };
@@ -1374,19 +1379,19 @@ bool f_libxml_use_internal_errors(CVarRef use_errors /* = null_variant */) {
 }
 
 static xmlParserInputBufferPtr
-hphp_libxml_input_buffer_noload(const char *URI, xmlCharEncoding enc) {
-  return nullptr;
+hphp_libxml_input_buffer(const char *URI, xmlCharEncoding enc) {
+  if (s_libxml_errors->m_entity_loader_disabled) {
+    return nullptr;
+  }
+  return __xmlParserInputBufferCreateFilename(URI, enc);
 }
 
 bool f_libxml_disable_entity_loader(bool disable /* = true */) {
-  xmlParserInputBufferCreateFilenameFunc old;
+  bool old = s_libxml_errors->m_entity_loader_disabled;
 
-  if (disable) {
-    old = xmlParserInputBufferCreateFilenameDefault(hphp_libxml_input_buffer_noload);
-  } else {
-    old = xmlParserInputBufferCreateFilenameDefault(nullptr);
-  }
-  return (old == hphp_libxml_input_buffer_noload);
+  s_libxml_errors->m_entity_loader_disabled = disable;
+
+  return old;
 }
 
 ///////////////////////////////////////////////////////////////////////////////

hphp/test/zend/good/ext/libxml/tests/libxml_disable_entity_loader.php

@@ -0,0 +1,25 @@
+<?php
+
+$xml = <<<EOT
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE test [<!ENTITY xxe SYSTEM "XXE_URI">]>
+<foo>&xxe;</foo>
+EOT;
+
+$xml = str_replace('XXE_URI', __DIR__ . '/libxml_disable_entity_loader_payload.txt', $xml);
+
+function parseXML($xml) {
+  $doc = new DOMDocument();
+  $doc->resolveExternals = true;
+  $doc->substituteEntities = true;
+  $doc->validateOnParse = false;
+  $doc->loadXML($xml, 0);
+  return $doc->saveXML();
+}
+
+var_dump(strpos(parseXML($xml), 'SECRET_DATA') !== false);
+var_dump(libxml_disable_entity_loader(true));
+var_dump(strpos(parseXML($xml), 'SECRET_DATA') === false);
+
+echo "Done\n";
+?>

hphp/test/zend/good/ext/libxml/tests/libxml_disable_entity_loader.php.expectf

@@ -0,0 +1,7 @@
+bool(true)
+bool(false)
+I/O warning : failed to load external entity "%s"
+HipHop Warning: %s
+HipHop Warning: %s
+bool(true)
+Done

hphp/test/zend/good/ext/libxml/tests/libxml_disable_entity_loader.php.skipif

@@ -0,0 +1 @@
+<?php if (!extension_loaded('simplexml') || !extension_loaded('dom') || defined('PHP_WINDOWS_VERSION_MAJOR')) die('skip'); ?>

hphp/test/zend/good/ext/libxml/tests/libxml_disable_entity_loader_payload.txt

@@ -0,0 +1 @@
+SECRET_DATA