fix(core): correct xml schema validation handling without needing external access by chadlwilson · Pull Request #8272 · dependency-check/DependencyCheck

chadlwilson · 2026-02-02T14:03:18Z

Description of Change

The changes in #8254 unfortunately exposed a few issues which this PR corrects which I alluded to at #8254 (comment)

The schemaLocation hints I added actually currently cause the default SAX parsers to try and load from the external source, via https:// :-(
The previous code was trying to force-enable javax.xml.accessExternalSchema to file, https which allowed this
- However the first schema to load, if done via a schemaLocation attribute would do so without external permissions; because the javax.xml.accessExternalSchema setting was being set after the SaxParserFactory was created. In practice, this means on master the hints schema would fail loading with the new schema location, since it is the first Analyzer prepared. (Caused by: org.xml.sax.SAXException: Line=5, Column=104: schema_reference: Failed to read schema document 'dependency-hint.1.4.xsd', because 'https' access is not allowed due to restriction set by the accessExternalSchema property.)
There was an attempt to avoid going out to the internet with a custom entity resolver, however we did not update this for the project/domain name change, so it wasn't working for https://dependency-check.github.io URLs - and also was only applied to suppression xsds.

Testing challenges

There was no testing/validation that the base hints can be parsed/loaded (resolved)
The tests to load base suppressions did so using a different API, without validation
There was a custom (old) Xerces version from 2006 on the classpath which was being used in preference to the bundled loader during tests; which did not impose the same security defaults. Fixed in test: avoid polluting test classpaths with sample dependencies to be scanned #8267

This change

retains the ability to validate XML docs by namespace without schema locations
removes the original xml javax.xml.accessExternalSchema system property mutation, by replacing with a mechanism to load https and file references from local resources by "convention".
- the system property setting was added originally in Error running depndency-check with Maven/Jenkins #2073 to handle folks setting schemaLocation (alongside an EntityResolver made available for suppression documents)
the new EntityResolver will inspect any attempts to resolve an entity for our own domain prefixes, and match against the the same file name as a passed resource.
- The only important coupling when publishing to GitHub pages is the filename should stay the same.
stops interfering with System properties for XML going forward. This wasn't really safe in a multi-threaded environment anyway, with something like a Gradle Daemon and other things running.
avoids any remote access while validating XML, except if the user has specifically enabled it for another reason to avoid us accidentally recreating such a situation
reviewed and consolidated the SAXParser creation to ensure they have the same security settings whether validating or not

Related issues

relates to docs: define schema locations in XML examples #8254

Have test cases been added to cover the new functionality?

yes

Copilot

Pull request overview

Fixes XML schema validation so that documents can be validated without triggering outbound network access, while preserving support for namespace-based validation and schemaLocation hints.

Changes:

Reworks secure SAX parsing/validation to use a validating XMLReader plus an entity resolver that maps known Dependency-Check schema URLs to local bundled XSD resources (no javax.xml.accessExternalSchema mutation).
Adds a new AutoCloseableInputSource abstraction and expands unit tests/resources to cover validating behavior (valid/invalid docs, schemaLocation variants, resolver behavior).
Refactors core parsers/tests to use the new XmlUtils APIs and removes Engine-level system property reset logic.

Reviewed changes

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

Show a summary per file

File	Description
utils/src/test/resources/schema-validation/simpledoc-valid-schemaloc-https.xml	Adds a schemaLocation-based validation fixture using the canonical HTTPS schema URL.
utils/src/test/resources/schema-validation/simpledoc-valid-schemaloc-https-legacy.xml	Adds a schemaLocation-based validation fixture using the legacy HTTPS schema URL.
utils/src/test/resources/schema-validation/simpledoc-valid-schemaloc-ambiguous-uri.xml	Adds a validation fixture for schemaLocation using an ambiguous/relative schema reference.
utils/src/test/resources/schema-validation/simpledoc-valid-no-schemaloc.xml	Adds a fixture validating by namespace without schemaLocation hints.
utils/src/test/resources/schema-validation/simpledoc-invalid-schemaloc-https-notfound.xml	Adds a negative fixture for disallowed/unknown HTTPS schemaLocation.
utils/src/test/resources/schema-validation/simpledoc-invalid-schemaloc-https-badprefix.xml	Adds a negative fixture for an untrusted HTTPS schemaLocation prefix.
utils/src/test/resources/schema-validation/simpledoc-invalid-schemaloc-file.xml	Adds a negative fixture for a `file://` schemaLocation under restricted external access.
utils/src/test/resources/schema-validation/simpledoc-invalid-schemaloc-ambiguous-uri.xml	Adds a negative fixture for an unresolvable ambiguous schemaLocation.
utils/src/test/resources/schema-validation/simpledoc-invalid-no-schemaloc-bad-content.xml	Adds a negative fixture for invalid XML content under a known schema.
utils/src/test/resources/schema-validation/simple.xsd	Adds a simple schema used by the XML validation tests.
utils/src/test/resources/schema-validation/irrelevant.xsd	Adds a non-matching schema used to verify “no relevant schema” behavior.
utils/src/test/java/org/owasp/dependencycheck/utils/XmlUtilsTest.java	Adds validation tests for `XmlUtils.buildSecureValidatingXmlReader` and the new resolver behavior.
utils/src/test/java/org/owasp/dependencycheck/utils/AutoCloseableInputSourceTest.java	Adds tests ensuring `AutoCloseableInputSource` closes underlying streams and sets systemId.
utils/src/main/java/org/owasp/dependencycheck/utils/XmlUtils.java	Introduces secure validating/non-validating `XMLReader` builders and a resolver that maps known schema URLs to local resources.
utils/src/main/java/org/owasp/dependencycheck/utils/FileUtils.java	Adjusts resource stream API annotations (resource stream is non-null / throws).
utils/src/main/java/org/owasp/dependencycheck/utils/AutoCloseableInputSource.java	Adds an `InputSource` wrapper that is `AutoCloseable` and provides `fromResource(...)`.
core/src/test/java/org/owasp/dependencycheck/xml/suppression/SuppressionHandlerTest.java	Updates suppression XML parsing test to use the secure validating `XMLReader`.
core/src/test/java/org/owasp/dependencycheck/xml/hints/HintHandlerTest.java	Updates hint XML parsing test to use the secure validating `XMLReader`.
core/src/test/java/org/owasp/dependencycheck/xml/assembly/GrokHandlerTest.java	Updates Grok assembly XML parsing test to use the secure validating `XMLReader`.
core/src/test/java/org/owasp/dependencycheck/resources/DependencyCheckBaseSuppressionTest.java	Removes the old DOM-based base suppression “base attribute” test (replaced by newer coverage elsewhere).
core/src/test/java/org/owasp/dependencycheck/analyzer/HintAnalyzerTest.java	Adds coverage ensuring core packaged hints are loadable via analyzer initialization/prepare.
core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java	Adds coverage ensuring base/hosted snapshot suppressions are marked as base and load correctly.
core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRule.java	Adds `equals`/`hashCode` support for suppression rule comparisons (used in new tests).
core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionParser.java	Switches suppression parsing to local XSD `InputSource`s + secure validating `XMLReader` and removes the old resolver.
core/src/main/java/org/owasp/dependencycheck/xml/pom/PomParser.java	Switches to the new non-validating secure `XMLReader` helper.
core/src/main/java/org/owasp/dependencycheck/xml/hints/HintParser.java	Switches hint parsing to local XSD `InputSource`s + secure validating `XMLReader`.
core/src/main/java/org/owasp/dependencycheck/xml/assembly/GrokParser.java	Switches grok parsing to local XSD `InputSource` + secure validating `XMLReader`.
core/src/main/java/org/owasp/dependencycheck/reporting/ReportGenerator.java	Uses secure `XMLReader` for XML pretty printing and adjusts template loading logic.
core/src/main/java/org/owasp/dependencycheck/data/cache/DataCacheFactory.java	Removes redundant null check for cache properties resource stream.
core/src/main/java/org/owasp/dependencycheck/analyzer/HintAnalyzer.java	Removes redundant null checks and exposes hint arrays for testing via `@VisibleForTesting`.
core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java	Removes redundant null check for embedded GrokAssembly zip stream.
core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java	Tightens hosted suppression URL filename handling and simplifies suppression file loading logic.
core/src/main/java/org/owasp/dependencycheck/Engine.java	Removes storing/restoring `javax.xml.accessExternalSchema` system property (no longer mutated).

Comments suppressed due to low confidence (1)

core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java:413

If GrokAssembly.zip is missing from the classpath, FileUtils.getResourceAsStream will throw FileNotFoundException which is currently caught by the generic IOException handler and rethrown as "Unable to create temp directory for GrokAssembly". Catch FileNotFoundException separately (before creating the temp dir) and throw an InitializationException that accurately reports the missing embedded resource.

        try (InputStream in = FileUtils.getResourceAsStream("GrokAssembly.zip")) {
            location = FileUtils.createTempDirectory(getSettings().getTempDirectory());
            ExtractionUtil.extractFiles(in, location);
        } catch (ExtractionException ex) {
            throw new InitializationException("Unable to extract GrokAssembly.dll", ex);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

For some time now, these streams have always been non-null Signed-off-by: Chad Wilson <[email protected]>

…g consistently Uses the actual analyzers to load them and ensure they can be parsed correctly Signed-off-by: Chad Wilson <[email protected]>

…hints, suppression rules and grok documents when used with schema locations Signed-off-by: Chad Wilson <[email protected]>

Copilot

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java:416

extractGrokAssembly() will wrap a missing GrokAssembly.zip resource (via FileUtils.getResourceAsStream) as “Unable to create temp directory for GrokAssembly”, because FileNotFoundException is caught by the generic IOException handler. Catch FileNotFoundException separately (or check resource existence up-front) and throw an InitializationException that accurately reports the missing GrokAssembly resource.

        try (InputStream in = FileUtils.getResourceAsStream("GrokAssembly.zip")) {
            location = FileUtils.createTempDirectory(getSettings().getTempDirectory());
            ExtractionUtil.extractFiles(in, location);
        } catch (ExtractionException ex) {
            throw new InitializationException("Unable to extract GrokAssembly.dll", ex);
        } catch (IOException ex) {
            throw new InitializationException("Unable to create temp directory for GrokAssembly", ex);
        }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jeremylong

lgtm

chadlwilson · 2026-02-03T13:15:21Z

Oh wow that was quick @jeremylong thanks! i was expecting more questions given how I managed to screw this up in the first place 😅

This exercise was a good reminder about how much I have always hated XML in Java, and that only WSDL was worse 😅

chadlwilson · 2026-02-03T13:16:06Z

The APIs really are terrible by modern standards, and Xerces/JAXP seems dead/dormant :(

boring-cyborg Bot added core changes to core tests test cases utils changes to utils labels Feb 2, 2026

chadlwilson requested a review from Copilot February 2, 2026 14:03

Copilot started reviewing on behalf of chadlwilson February 2, 2026 14:04 View session

chadlwilson changed the title ~~fix(core): correct xml schema validation handling without external access~~ fix(core): correct xml schema validation handling without needing external access Feb 2, 2026

Copilot AI reviewed Feb 2, 2026

View reviewed changes

Comment thread core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRule.java

chadlwilson force-pushed the fix-xml-schema-handling branch from eb901c8 to 71fe68f Compare February 2, 2026 15:05

chadlwilson requested a review from Copilot February 2, 2026 15:05

Copilot AI reviewed Feb 2, 2026

View reviewed changes

chadlwilson force-pushed the fix-xml-schema-handling branch from 71fe68f to 510038e Compare February 2, 2026 15:28

chadlwilson requested a review from Copilot February 2, 2026 15:29

Copilot started reviewing on behalf of chadlwilson February 2, 2026 15:29 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

Comment thread core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java

Comment thread core/src/main/java/org/owasp/dependencycheck/reporting/ReportGenerator.java

chadlwilson added 3 commits February 2, 2026 23:53

chore: Remove unnecessary null checks

f07d729

For some time now, these streams have always been non-null Signed-off-by: Chad Wilson <[email protected]>

test: Ensure packaged suppressions and hints are validated for loadin…

0d8dfcc

…g consistently Uses the actual analyzers to load them and ensure they can be parsed correctly Signed-off-by: Chad Wilson <[email protected]>

fix(core): correct schema validation to avoid external access across …

a834dbf

…hints, suppression rules and grok documents when used with schema locations Signed-off-by: Chad Wilson <[email protected]>

chadlwilson force-pushed the fix-xml-schema-handling branch from 510038e to a834dbf Compare February 2, 2026 15:54

chadlwilson requested a review from Copilot February 2, 2026 15:54

Copilot started reviewing on behalf of chadlwilson February 2, 2026 15:54 View session

Copilot AI reviewed Feb 2, 2026

View reviewed changes

Comment thread core/src/main/java/org/owasp/dependencycheck/reporting/ReportGenerator.java

Comment thread utils/src/main/java/org/owasp/dependencycheck/utils/XmlUtils.java

Comment thread core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionRule.java

jeremylong approved these changes Feb 3, 2026

View reviewed changes

jeremylong merged commit c82a59c into dependency-check:main Feb 3, 2026
10 of 11 checks passed

jeremylong added this to the 12.2.1 milestone Feb 3, 2026

chadlwilson deleted the fix-xml-schema-handling branch February 3, 2026 13:13

github-actions Bot locked as resolved and limited conversation to collaborators Mar 6, 2026

Conversation

chadlwilson commented Feb 2, 2026

Description of Change

Related issues

Have test cases been added to cover the new functionality?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jeremylong left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chadlwilson commented Feb 3, 2026

Uh oh!

chadlwilson commented Feb 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants