Skip to content

Add defense-in-depth XML secure processing to SAXSource and StAXSource paths - #2915

Merged
Muskan Gupta (muskan124947) merged 6 commits into
mainfrom
users/muskgupta/fix_xml_xxe_attacks
Mar 24, 2026
Merged

Muskan Gupta (muskan124947) merged 6 commits into
mainfrom
users/muskgupta/fix_xml_xxe_attacks

Conversation

@muskan124947

@muskan124947 Muskan Gupta (muskan124947) commented Mar 23, 2026 •

Copy link
Copy Markdown
Contributor

The getDOMSource() method in SQLServerSQLXML.java already sets
FEATURE_SECURE_PROCESSING and uses a custom EntityResolver. This change
applies equivalent protections to getSAXSource() and getStAXSource() for
consistency:

  • getSAXSource(): enable FEATURE_SECURE_PROCESSING and disallow-doctype-decl
  • getStAXSource(): disable IS_SUPPORTING_EXTERNAL_ENTITIES and SUPPORT_DTD

SQL Server's xml data type rejects DTDs at the storage layer, so these
are defense-in-depth safeguards to ensure all three parser paths are
uniformly hardened.

Copilot AI 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.

Pull request overview

Adds defense-in-depth XML secure-processing configuration when returning SAXSource and StAXSource from SQLServerSQLXML, aligning these code paths more closely with the existing hardening in getDOMSource().

Changes:

  • Configure SAXParserFactory with secure-processing and DOCTYPE-disallow features for SAXSource.
  • Configure XMLInputFactory to disable external entities and DTD support for StAXSource.

@codecov

codecov Bot commented Mar 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 60.59%. Comparing base (415984c) to head (f957080).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
.../com/microsoft/sqlserver/jdbc/SQLServerSQLXML.java 0.00% 17 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2915      +/-   ##
============================================
- Coverage     60.61%   60.59%   -0.03%     
+ Complexity     4911     4910       -1     
============================================
  Files           151      151              
  Lines         35000    35016      +16     
  Branches       5854     5854              
============================================
+ Hits          21216    21218       +2     
- Misses        10939    10952      +13     
- Partials       2845     2846       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI 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.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Copilot AI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

@muskan124947
Muskan Gupta (muskan124947) merged commit 9978575 into main Mar 24, 2026
17 of 19 checks passed
@github-project-automation github-project-automation Bot moved this from In progress to Closed/Merged PRs in MSSQL JDBC Mar 24, 2026
@muskan124947 Muskan Gupta (muskan124947) added this to the 13.5.1 milestone Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Closed/Merged PRs

Development

Successfully merging this pull request may close these issues.

5 participants