Potential fix for code scanning alert no. 146: Resolving XML external entity in user-controlled data

[AndresMaqueo]

Potential fix for https://github.com/AndresMaqueo/ambari/security/code-scanning/146

In general, to fix XXE vulnerabilities in Java code that uses JAXP/StAX/JAXB, you must configure the XML parser to disallow DTDs and external entities before parsing any untrusted XML. For XMLInputFactory/XMLStreamReader, this means turning off DTD support and entity resolution features. For SchemaFactory, you should also enable the JAXP secure processing feature and, where possible, disable access to external DTDs and schemas.

For this specific issue, the best fix with minimal functional change is to harden the XMLInputFactory instance created in VersionDefinitionXml.load(InputStream stream) and to harden the SchemaFactory used for XSD validation. We should set the standard and implementation-specific properties recommended by OWASP:

• On XMLInputFactory:

  • Disable DTD support: XMLInputFactory.SUPPORT_DTD → false
  • Disable external entities: "javax.xml.stream.isSupportingExternalEntities" → false
  • Optionally, enable namespace awareness (already default in many impls, but harmless).

• On SchemaFactory:

  • Enable secure processing: XMLConstants.FEATURE_SECURE_PROCESSING → true
  • Disable external DTD and schema access (if supported by the JAXP implementation) via setProperty:
    • "http://apache.org/xml/properties/accessExternalDTD" → ""
    • "http://apache.org/xml/properties/accessExternalSchema" → ""

These changes only affect how XML is parsed and validated; they do not change the higher-level behavior of reading version definition files, except that malicious XML relying on DTDs or external entities will now fail to parse. No changes are required in URLRedirectProvider or VersionDefinitionResourceProvider; they continue to pass strings/streams as before, but the parser is now safe.

Concretely, in VersionDefinitionXml.load(InputStream stream), after creating xmlFactory, configure its anti-XXE properties before creating the XMLStreamReader. Also, after creating the SchemaFactory, configure secure processing and disable external access before creating the Schema. We can keep all imports (no new ones are needed, since we already import XMLConstants, XMLInputFactory, and SchemaFactory).

Suggested fixes powered by Copilot Autofix. Review carefully before merging.

Summary by CodeRabbit

• Bug Fixes
  • Enhanced security in XML parsing operations by restricting unnecessary external resource access.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

The changes introduce XXE (XML External Entity) security mitigations in the XML parsing workflow of VersionDefinitionXml. XMLInputFactory and SchemaFactory are configured to disable DTD support, external entities, and external schema access, with graceful error handling for unsupported properties.

Changes

Cohort / File(s)	Summary
XXE Security Hardening ambari-server/src/main/java/org/apache/ambari/server/state/repository/VersionDefinitionXml.java	Configures XMLInputFactory and SchemaFactory in load(InputStream) to disable DTD, external entities, and external schema access; adds guarded property setting with tolerant error handling

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A hop through XML's dark forest deep,
Where external entities once did creep,
Now DTDs are locked away,
XXE attacks shall not prey,
Security bunny's promise to keep! 🔒

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 945422b and 8880617.

📒 Files selected for processing (1)

• ambari-server/src/main/java/org/apache/ambari/server/state/repository/VersionDefinitionXml.java

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[AndresMaqueo]

This PR hardens XML parsing against XXE as detected by CodeQL alert apache#146.
The change is intentionally minimal and limited to parser configuration.
All CodeQL checks pass and the alert is resolved.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".