disable external entity resolution in XML parsers#284
Open
digi-scrypt wants to merge 1 commit into
Open
Conversation
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
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.
DOMParser, JDOMParser and the deprecated XMLDocumentContainer transform path parse XML from a caller-supplied URL/Source without turning off external entities, so a document carrying an external general entity like gets resolved at parse time and its contents land in the tree, which is the usual file-read / SSRF XXE. The fix turns off external general/parameter entities and external DTD loading inside each parser while leaving the internal DTD subset working, so the existing Vendor.xml style documents still parse. One thing I went back and forth on: for JDOM the SAX feature flags alone are not enough because setExpandEntities(true) re-enables external general entities afterward, so I refuse external lookups with a no-op EntityResolver there instead.