Security: harden user-import XML parsing against XXE#8609
Merged
Conversation
…2wwx) Centralize XML parsing for the user-import flows in a single Import::xml()/xmlFromString() helper that hardens against XXE by blocking external entity loading via libxml_set_external_entity_loader, restoring the default loader in a finally block. This mirrors the 1.11.x Import::xml() approach but uses the modern API (libxml_disable_entity_loader is deprecated and a no-op on PHP 8.x; LIBXML_NOENT would make things worse by enabling entity expansion). Refactors parse_xml_data in myspace.lib.php, user_import.php and user_update_import.php to call Import::xml($file). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AngelFQC
force-pushed
the
security/auto-fix-GHSA-h24x-xw47-2wwx
branch
from
393590a to
e38ea6e
Compare
Apply the same XXE hardening to the other XML import surfaces found while auditing Crawler/loadXML usage: - scorm.class.php (SCORM manifest) and exercise_import.inc.php (QTI2) now parse through the centralized Import::xmlFromString() helper instead of a bare Crawler->addXmlContent(). - Moodle backup and Common Cartridge importers (src/CourseBundle) now pass LIBXML_NONET to DOMDocument::loadXML(), matching the convention already used by the SVG and legacy Moodle parsers. PDF (HTML) and the LIBXML_NONET-protected simplexml parsers were reviewed and are not affected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The user import and update features parse an admin-uploaded XML file through
Crawler::addXmlContent(file_get_contents($file))without any explicit XML hardening. Protection against external-entity resolution was left entirely to the libxml runtime default, so on PHP < 8 or a misconfigured/downgraded environment the parser could resolve external entities and disclose local files.Fix
Wrap each user-import XML parse with an explicit, non-deprecated guard that blocks external entity loading regardless of the runtime default, then restores the default loader:
Applied at all three user-import parse sites:
myspace.lib.php::parse_xml_data(),admin/user_import.phpandadmin/user_update_import.php.Notes:
libxml_disable_entity_loader()is deprecated on PHP 8.x, so the modernlibxml_set_external_entity_loader()is used instead.LIBXML_NOENTis intentionally not used — it would enable entity substitution and make the situation worse. Symfony'saddXmlContent()keeps its safeLIBXML_NONETdefault.Invariant now enforced
User-import XML parsing never resolves external entities (e.g.
file:///…), independent of the PHP/libxml version or configuration.⚠ Scope note
lp/scorm.class.phpparses uploaded SCORM XML through the sameaddXmlContentpattern; it is a different feature outside this advisory's scope and was left untouched. It would benefit from the same hardening as a follow-up.OWASP control
A05:2021 – Security Misconfiguration (CWE-611, XML External Entity).
Refs GHSA-h24x-xw47-2wwx
🤖 Generated with Claude Code