Skip to content

fix(xml): prevent XXE in xml.xpath (disable DTD + external entities) - #357

Open
yogikortisa wants to merge 1 commit into
google:masterfrom
yogikortisa:fix/xmlmodule-xxe
Open

yogikortisa wants to merge 1 commit into
google:masterfrom
yogikortisa:fix/xmlmodule-xxe

Conversation

@yogikortisa

Copy link
Copy Markdown

Summary

XmlModule.compile() — the Starlark xml.xpath(content, expression, type) function — parses the provided content with a default DocumentBuilderFactory:

DocumentBuilderFactory builderFactory = DocumentBuilderFactory.newInstance();
DocumentBuilder builder = builderFactory.newDocumentBuilder();
Document xmlDocument = builder.parse(new ByteArrayInputStream(xmlContent.getBytes(...)));

A default DocumentBuilderFactory resolves DTDs and external entities. When the XML content comes from an untrusted source — for example a file read from a repository being migrated — a document such as:

<!DOCTYPE x [ <!ENTITY e SYSTEM "file:///etc/passwd"> ]>
<root>&e;</root>

enables XML External Entity (XXE) attacks on the host running Copybara: local file disclosure, server-side request forgery (http://… entities), and denial of service via entity expansion.

Fix

Disable DTD processing (disallow-doctype-decl, which also blocks entity resolution), XInclude, and entity-reference expansion before constructing the parser. XPath over normal XML content is unaffected.

XmlModule.compile() (the Starlark xml.xpath function) parsed the provided XML
content with a default DocumentBuilderFactory, which resolves DTDs and external
entities. When the content originates from an untrusted source (e.g. a file in
a migrated repository), a document such as:

  <!DOCTYPE x [<!ENTITY e SYSTEM "file:///etc/passwd">]>

enables XML External Entity (XXE) attacks: local file disclosure, SSRF, and
denial of service via entity expansion.

Disable DTD processing (disallow-doctype-decl), XInclude, and entity-reference
expansion before building the parser.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the XmlModule.java file to secure the XML parser against XML External Entity (XXE) injection. It configures the DocumentBuilderFactory to disallow doctype declarations, disable XInclude, and disable expanding entity references. No review comments were provided, so there is no feedback to address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant