Issue #1816136 by Berdir, David_Rothstein, ubercomp, c960657, wojtha, fgm,...

Issue #1816136 by Berdir, David_Rothstein, ubercomp, c960657, wojtha, fgm, pwolanin, Damien Tournoud, Heine: Fixed OpenID vulnerability from SA-CORE-2012-003 (and backport anything to 7.x-dev as necessary).

core/modules/openid/lib/Drupal/openid/Tests/OpenIDFunctionalTest.php

@@ -185,6 +185,16 @@ function testLogin() {
     $this->assertLink(t('Log out'), 0, 'User was logged in.');
     $response = variable_get('openid_test_hook_openid_response_response');
     $this->assertEqual($response['openid.identity'], $new_identity, 'hook_openid_request_alter() were invoked.');
+
+    $this->drupalLogout();
+
+    // Use a User-supplied Identity that is the URL of an XRDS document.
+    // Tell the test module to add a doctype. This should fail.
+    $identity = url('openid-test/yadis/xrds', array('absolute' => TRUE, 'query' => array('doctype' => 1)));
+    // Test logging in via the login block on the front page.
+    $edit = array('openid_identifier' => $identity);
+    $this->drupalPost('', $edit, t('Log in'), array(), array(), 'openid-login-form');
+    $this->assertRaw(t('Sorry, that is not a valid OpenID. Ensure you have spelled your ID correctly.'), 'XML with DOCTYPE was rejected.');
   }
 
   /**

core/modules/openid/openid.inc

@@ -133,8 +133,24 @@ function openid_redirect_form($form, &$form_state, $url, $message) {
  */
 function _openid_xrds_parse($raw_xml) {
   $services = array();
-  try {
-    $xml = @new SimpleXMLElement($raw_xml);
+
+  // Protect against malicious doctype declarations and other unexpected entity
+  // loading.
+  $load_entities = libxml_disable_entity_loader(TRUE);
+
+  // Load the XML into a DOM document.
+  $dom = new DOMDocument();
+  @$dom->loadXML($raw_xml);
+
+  // Since DOCTYPE declarations from an untrusted source could be malicious, we
+  // stop parsing here and treat the XML as invalid since XRDS documents do not
+  // require, and are not expected to have, a DOCTYPE.
+  if (isset($dom->doctype)) {
+    return array();
+  }
+
+  // Parse the DOM document for the information we need.
+  if ($xml = simplexml_import_dom($dom)) {
     foreach ($xml->children(OPENID_NS_XRD)->XRD as $xrd) {
       foreach ($xrd->children(OPENID_NS_XRD)->Service as $service_element) {
         $service = array(
@@ -160,9 +176,10 @@ function _openid_xrds_parse($raw_xml) {
       }
     }
   }
-  catch (Exception $e) {
-    // Invalid XML.
-  }
+
+  // Return the LIBXML options to the previous state before returning.
+  libxml_disable_entity_loader($load_entities);
+
   return $services;
 }
 

core/modules/openid/tests/openid_test.module

@@ -101,7 +101,11 @@ function openid_test_yadis_xrds() {
     if (arg(3) == 'xri' && (arg(4) != '@example*résumé;%25' || $_GET['_xrd_r'] != 'application/xrds xml')) {
       throw new NotFoundHttpException();
     }
-    $output = '<?xml version="1.0" encoding="UTF-8"?>
+    $output = '<?xml version="1.0" encoding="UTF-8"?>';
+    if (!empty($_GET['doctype'])) {
+      $output .= "\n<!DOCTYPE dct [ <!ELEMENT blue (#PCDATA)> ]>\n";
+    }
+    $output .= '
       <xrds:XRDS xmlns:xrds="xri://$xrds" xmlns="xri://$xrd*($v*2.0)" xmlns:openid="http://openid.net/xmlns/1.0">
         <XRD>
           <Status cid="' . check_plain(variable_get('openid_test_canonical_id_status', 'verified')) . '"/>