Skip to content

RAT-560: changes to reduce XXE exposure#679

Merged
ottlinger merged 13 commits into
masterfrom
fix-xxe-issues
Jun 17, 2026
Merged

RAT-560: changes to reduce XXE exposure#679
ottlinger merged 13 commits into
masterfrom
fix-xxe-issues

Conversation

@Claudenw

@Claudenw Claudenw commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

RAT-560: Static code analysis (also GitHub security scans) brings up potential XXE attacks in our XMLParser configuration:

  • Disable access to external entities in XML parsing.

@Claudenw Claudenw requested a review from ottlinger June 16, 2026 15:49
@ottlinger ottlinger changed the title changes to reduce XXE exposure RAT-560: changes to reduce XXE exposure Jun 16, 2026
@ottlinger

Copy link
Copy Markdown
Contributor

@Claudenw having a look at the coverage report I wonder if it makes sense to add a test that defines a custom stylesheet to transform the report ..... or should we just merge the PR?

https://sonarcloud.io/code?id=apache_creadur-rat&pullRequest=679&selected=apache_creadur-rat%3Aapache-rat-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Frat%2Futils%2FStandardXmlFactory.java&line=70

I guess it still wouldn't give us another 4% coverage. WDYT?

@ottlinger ottlinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM - added minor changes and a little more test. Thanks for clarification and adding protection against XXE attacks in that regard.

@ottlinger

Copy link
Copy Markdown
Contributor

@Claudenw the tests seem to be problematic as they have different results in different languages:

[ERROR] Failures: 
[ERROR]   StandardXmlFactoryTests.noEmptyInput:48 
Expecting throwable message:
  "Vorzeitiges Dateiende."
to contain:
  "Premature end of file."
but did not.

Throwable that failed the check:

javax.xml.transform.TransformerConfigurationException: Vorzeitiges Dateiende.

@Claudenw

Copy link
Copy Markdown
Contributor Author

@ottlinger I removed the test for the translated text.

@sonarqubecloud

Copy link
Copy Markdown

@Claudenw

Copy link
Copy Markdown
Contributor Author

@ottlinger Are we good to merge this?

@ottlinger ottlinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@ottlinger ottlinger merged commit 4552426 into master Jun 17, 2026
27 checks passed
@ottlinger ottlinger deleted the fix-xxe-issues branch June 17, 2026 19:40
@ottlinger

Copy link
Copy Markdown
Contributor

@Claudenw thanks, feel free to close the corresponding Jira ticket RAT-560 in case that's all here.

potiuk added a commit to potiuk/creadur-rat that referenced this pull request Jun 21, 2026
…d (external entities disabled, apache#679 hardens DOCTYPE), no-network confirmed (XSLT xsl:include caveat), correct archive path-handling (read to memory, no extract-to-disk → no path traversal), Whisker/Tentacles deferred
potiuk added a commit to potiuk/creadur-rat that referenced this pull request Jun 27, 2026
…e labels

Answers Claudenw's review note (does apache#679 impact the XXE data-flow line?):
the §5a/§8 text already records that RAT disables external entities + the
apache#679 DOCTYPE hardening, but the data-flow diagram and the input/residual
tables still labelled XXE a bare "surface". Annotate those three labels with
the mitigation so the diagram is consistent with §5a/§8 apache#2.

Generated-by: Claude Opus 4.8 (1M context)
potiuk added a commit to potiuk/creadur-rat that referenced this pull request Jun 27, 2026
Consistency with THREAT_MODEL.md (§5a / §8 apache#2): since RAT-560 (apache#679) RAT
builds XML parsers via the hardened StandardXmlFactory (DOCTYPE + external
entities disabled), so XXE is actively prevented. Lead with that; keep the
operator-trusted-config argument as defense-in-depth.

Generated-by: Claude Opus 4.8 (1M context)
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.

2 participants