← Back to team overview

mahara-contributors team mailing list archive

[Bug 1340151] Re: Consider a different approach to libxml_disable_entity_loader(true) in init.php

 

Dan helpfully posted a bunch of links in the bug description showing how
other products have addressed this problem. I find the Mediawiki one to
be the most concise and explanatory:
http://www.mediawiki.org/wiki/XML_External_Entity_Processing

Based on that, we need this plan of action:

1. Remove the part from init.php where we do
libxml_disable_entity_loader(true);

2. Provide wrapper functions for DOMDocument -> loadXML() and
simplexml_load_string(), which will run libxml_disable_entity_loader()
before the vulnerable code, and then set it back to the old value
immediately after.

3. Replace all existing calls to DOMDocument -> loadXML() and
simplexml_load_string() with calls to our wrapper functions.

XMLReader->XML() is also vulnerable, but we don't use that in the Mahara
codebase anywhere.

We'll need to test this to make sure that it no longer crashes Moodle,
*and* that it doesn't leave us vulnerable to the initual security issue
we were trying to fix.

** Also affects: mahara/1.8
   Importance: Undecided
       Status: New

** Also affects: mahara/1.9
   Importance: Undecided
       Status: New

** Changed in: mahara/1.8
   Importance: Undecided => High

** Changed in: mahara/1.9
   Importance: Undecided => High

** Changed in: mahara/1.8
       Status: New => Confirmed

** Changed in: mahara/1.9
       Status: New => Confirmed

** Changed in: mahara/1.8
    Milestone: None => 1.8.6

** Changed in: mahara/1.9
    Milestone: None => 1.9.4

** Changed in: mahara/1.8
     Assignee: (unassigned) => Aaron Wells (u-aaronw)

** Changed in: mahara/1.9
     Assignee: (unassigned) => Aaron Wells (u-aaronw)

-- 
You received this bug notification because you are a member of Mahara
Contributors, which is subscribed to Mahara.
Matching subscriptions: Subscription for all Mahara Contributors -- please ask on #mahara-dev or mahara.org forum before editing or unsubscribing it!
https://bugs.launchpad.net/bugs/1340151

Title:
  Consider a different approach to libxml_disable_entity_loader(true) in
  init.php

Status in Mahara ePortfolio:
  Confirmed
Status in Mahara 1.10 series:
  Confirmed
Status in Mahara 1.8 series:
  Confirmed
Status in Mahara 1.9 series:
  Confirmed
Status in Mahara 15.04 series:
  Confirmed

Bug description:
  Unfortunately it seems like using libxml_disable_entity_loader(true)
  in init.php is unkind to other applications living on the same system.

  PHP Bug https://bugs.php.net/bug.php?id=64938 is the heart of the
  problem  - the use of this setting leaks between different threads and
  by setting it for the duration of every single Mahara request this bug
  comes into play much more easily.

  The other problem is https://bugs.php.net/bug.php?id=62577 which means
  that  simplexml_load_file() will not even load *local* files off disk.
  For example, this would break in Mahara even though no entities come
  into play:

  $xml = simplexml_load_file(get_config('libroot').'/db/install.xml');

  In Moodle we've been warned on one of our issues that users have seen
  this problem in the wild with Mahara, I didn't find an issue reported
  to you about it, so i'll copy and paste the report from our tracker
  here:

  "We had experience with this problem on an upgrade to Mahara 1.7, when
  https://bugs.launchpad.net/mahara/+bug/1047111 was fixed. The approach
  was the same as is suggested in this ticket. This caused seemingly
  random failures in Moodle (which is run on the same webserver)
  whenever there was usage of Mahara. The root cause was not obvious,
  nor well advertised with the Mahara fix, so it took awhile to finally
  implement a fix. In the meantime, significant functionality is broken.
  Unfortunately, there's really no way around this unless you switch
  from using mod_php."

  Yesterday whilst debugging some code with Yuliya we realised she also
  was encountering this horrible combination of bugs affecting her
  Moodle install so it seems this is not a theoretical problem.

  In Moodle we have decided to use the same approach of many other projects and enable/disable the loader around vulnerable code to reduce the chance of this combination of bugs affecting other applications or uses of the the simplexml_load_file:
  http://cgit.drupalcode.org/drupal/commit/?id=b912710
  https://github.com/symfony/symfony/blob/8ef8a1d289a6ce454b7c79baeddbfb45e4af6191/src/Symfony/Component/Config/Util/XmlUtils.php#L41
  http://www.mediawiki.org/wiki/XML_External_Entity_Processing

To manage notifications about this bug go to:
https://bugs.launchpad.net/mahara/+bug/1340151/+subscriptions


References