Skip to content

Commit a834dbf

Browse files
chadlwilsonchadlwilson
committed
fix(core): correct schema validation to avoid external access across hints, suppression rules and grok documents when used with schema locations
Signed-off-by: Chad Wilson <29788154+chadlwilson@users.noreply.github.com>
1 parent 0d8dfcc commit a834dbf

27 files changed

+555
-200
lines changed

‎core/src/main/java/org/owasp/dependencycheck/Engine.java‎

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -140,14 +140,6 @@ public class Engine implements FileFilter, AutoCloseable {
140140
* A reference to the database.
141141
*/
142142
private CveDB database = null;
143-
/**
144-
* Used to store the value of
145-
* System.getProperty("javax.xml.accessExternalSchema") - ODC may change the
146-
* value of this system property at runtime. We store the value to reset the
147-
* property to its original value.
148-
*/
149-
private final String accessExternalSchema;
150-
151143
/**
152144
* Creates a new {@link Mode#STANDALONE} Engine.
153145
*
@@ -188,8 +180,6 @@ public Engine(@NotNull final ClassLoader serviceClassLoader, @NotNull final Mode
188180
this.settings = settings;
189181
this.serviceClassLoader = serviceClassLoader;
190182
this.mode = mode;
191-
this.accessExternalSchema = System.getProperty("javax.xml.accessExternalSchema");
192-
193183
initializeEngine();
194184
}
195185

@@ -215,11 +205,6 @@ public void close() {
215205
database = null;
216206
}
217207
}
218-
if (accessExternalSchema != null) {
219-
System.setProperty("javax.xml.accessExternalSchema", accessExternalSchema);
220-
} else {
221-
System.clearProperty("javax.xml.accessExternalSchema");
222-
}
223208
JCS.shutdown();
224209
}
225210

‎core/src/main/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,7 +264,7 @@ private void loadHostedSuppressionBaseData(final SuppressionParser parser, final
264264
final URL url = new URL(configuredUrl);
265265
final String fileName = new File(url.getPath()).getName();
266266
if (fileName.isBlank()) {
267-
throw new IllegalArgumentException("Hosted Suppression URL must imply a filename");
267+
throw new IOException("Hosted Suppression URL must imply a filename");
268268
}
269269
final File repoFile = new File(getSettings().getDataDirectory(), fileName);
270270
boolean repoEmpty = !repoFile.isFile() || repoFile.length() <= 1L;

‎core/src/main/java/org/owasp/dependencycheck/xml/assembly/GrokParser.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,8 @@
2727
import java.nio.charset.StandardCharsets;
2828
import javax.annotation.concurrent.ThreadSafe;
2929
import javax.xml.parsers.ParserConfigurationException;
30-
import javax.xml.parsers.SAXParser;
3130

32-
import org.owasp.dependencycheck.utils.FileUtils;
31+
import org.owasp.dependencycheck.utils.AutoCloseableInputSource;
3332
import org.owasp.dependencycheck.utils.XmlUtils;
3433

3534
import org.slf4j.Logger;
@@ -38,6 +37,8 @@
3837
import org.xml.sax.SAXException;
3938
import org.xml.sax.XMLReader;
4039

40+
import static org.owasp.dependencycheck.utils.AutoCloseableInputSource.fromResource;
41+
4142
/**
4243
* A simple validating parser for XML Grok Assembly XML files.
4344
*
@@ -80,10 +81,9 @@ public AssemblyData parse(File file) throws GrokParseException {
8081
* @throws GrokParseException thrown if the XML cannot be parsed
8182
*/
8283
public AssemblyData parse(InputStream inputStream) throws GrokParseException {
83-
try (InputStream schema = FileUtils.getResourceAsStream(GROK_SCHEMA)) {
84+
try (AutoCloseableInputSource schema = fromResource(GROK_SCHEMA)) {
8485
final GrokHandler handler = new GrokHandler();
85-
final SAXParser saxParser = XmlUtils.buildSecureSaxParser(schema);
86-
final XMLReader xmlReader = saxParser.getXMLReader();
86+
final XMLReader xmlReader = XmlUtils.buildSecureValidatingXmlReader(schema);
8787
xmlReader.setErrorHandler(new GrokErrorHandler());
8888
xmlReader.setContentHandler(handler);
8989
try (Reader reader = new InputStreamReader(inputStream, StandardCharsets.UTF_8)) {

‎core/src/main/java/org/owasp/dependencycheck/xml/hints/HintParser.java‎

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,11 @@
2828
import java.util.List;
2929
import javax.annotation.concurrent.NotThreadSafe;
3030
import javax.xml.parsers.ParserConfigurationException;
31-
import javax.xml.parsers.SAXParser;
31+
3232
import org.apache.commons.io.ByteOrderMark;
3333
import org.apache.commons.io.input.BOMInputStream;
3434

35-
import org.owasp.dependencycheck.utils.FileUtils;
35+
import org.owasp.dependencycheck.utils.AutoCloseableInputSource;
3636
import org.owasp.dependencycheck.utils.XmlUtils;
3737

3838
import org.slf4j.Logger;
@@ -41,6 +41,8 @@
4141
import org.xml.sax.SAXException;
4242
import org.xml.sax.XMLReader;
4343

44+
import static org.owasp.dependencycheck.utils.AutoCloseableInputSource.fromResource;
45+
4446
/**
4547
* A simple validating parser for XML Hint Rules.
4648
*
@@ -53,21 +55,6 @@ public class HintParser {
5355
* The logger.
5456
*/
5557
private static final Logger LOGGER = LoggerFactory.getLogger(HintParser.class);
56-
/**
57-
* JAXP Schema Language. Source:
58-
* http://docs.oracle.com/javase/tutorial/jaxp/sax/validation.html
59-
*/
60-
public static final String JAXP_SCHEMA_LANGUAGE = "http://java.sun.com/xml/jaxp/properties/schemaLanguage";
61-
/**
62-
* W3C XML Schema. Source:
63-
* http://docs.oracle.com/javase/tutorial/jaxp/sax/validation.html
64-
*/
65-
public static final String W3C_XML_SCHEMA = "http://www.w3.org/2001/XMLSchema";
66-
/**
67-
* JAXP Schema Source. Source:
68-
* http://docs.oracle.com/javase/tutorial/jaxp/sax/validation.html
69-
*/
70-
public static final String JAXP_SCHEMA_SOURCE = "http://java.sun.com/xml/jaxp/properties/schemaSource";
7158

7259
/**
7360
* The schema for the hint XML files.
@@ -142,20 +129,18 @@ public void parseHints(File file) throws HintParseException {
142129
* @throws SAXException thrown if the XML cannot be parsed
143130
*/
144131
public void parseHints(InputStream inputStream) throws HintParseException, SAXException {
145-
try (
146-
InputStream schemaStream14 = FileUtils.getResourceAsStream(HINT_SCHEMA_1_4);
147-
InputStream schemaStream13 = FileUtils.getResourceAsStream(HINT_SCHEMA_1_3);
148-
InputStream schemaStream12 = FileUtils.getResourceAsStream(HINT_SCHEMA_1_2);
149-
InputStream schemaStream11 = FileUtils.getResourceAsStream(HINT_SCHEMA_1_1)) {
132+
try (AutoCloseableInputSource schemaStream14 = fromResource(HINT_SCHEMA_1_4);
133+
AutoCloseableInputSource schemaStream13 = fromResource(HINT_SCHEMA_1_3);
134+
AutoCloseableInputSource schemaStream12 = fromResource(HINT_SCHEMA_1_2);
135+
AutoCloseableInputSource schemaStream11 = fromResource(HINT_SCHEMA_1_1)) {
150136

151137
final BOMInputStream bomStream = BOMInputStream.builder().setInputStream(inputStream).get();
152138
final ByteOrderMark bom = bomStream.getBOM();
153139
final String defaultEncoding = StandardCharsets.UTF_8.name();
154140
final String charsetName = bom == null ? defaultEncoding : bom.getCharsetName();
155141

156142
final HintHandler handler = new HintHandler();
157-
final SAXParser saxParser = XmlUtils.buildSecureSaxParser(schemaStream14, schemaStream13, schemaStream12, schemaStream11);
158-
final XMLReader xmlReader = saxParser.getXMLReader();
143+
final XMLReader xmlReader = XmlUtils.buildSecureValidatingXmlReader(schemaStream14, schemaStream13, schemaStream12, schemaStream11);
159144
xmlReader.setErrorHandler(new HintErrorHandler());
160145
xmlReader.setContentHandler(handler);
161146
try (Reader reader = new InputStreamReader(bomStream, charsetName)) {

‎core/src/main/java/org/owasp/dependencycheck/xml/pom/PomParser.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,7 @@ public Model parseWithoutDocTypeCleanup(File file) throws PomParseException {
107107
public Model parse(InputStream inputStream) throws PomParseException {
108108
try {
109109
final PomHandler handler = new PomHandler();
110-
final SAXParser saxParser = XmlUtils.buildSecureSaxParser();
111-
final XMLReader xmlReader = saxParser.getXMLReader();
110+
final XMLReader xmlReader = XmlUtils.buildSecureXmlReader();
112111
xmlReader.setContentHandler(handler);
113112

114113
final BOMInputStream bomStream = BOMInputStream.builder()
@@ -138,8 +137,7 @@ public Model parse(InputStream inputStream) throws PomParseException {
138137
public Model parseWithoutDocTypeCleanup(InputStream inputStream) throws PomParseException {
139138
try {
140139
final PomHandler handler = new PomHandler();
141-
final SAXParser saxParser = XmlUtils.buildSecureSaxParser();
142-
final XMLReader xmlReader = saxParser.getXMLReader();
140+
final XMLReader xmlReader = XmlUtils.buildSecureXmlReader();
143141
xmlReader.setContentHandler(handler);
144142

145143
final BOMInputStream bomStream = BOMInputStream.builder().setInputStream(new XmlInputStream(inputStream)).get();

‎core/src/main/java/org/owasp/dependencycheck/xml/suppression/SuppressionParser.java‎

Lines changed: 10 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -28,20 +28,21 @@
2828
import java.util.List;
2929
import javax.annotation.concurrent.ThreadSafe;
3030
import javax.xml.parsers.ParserConfigurationException;
31-
import javax.xml.parsers.SAXParser;
31+
3232
import org.apache.commons.io.ByteOrderMark;
3333
import org.apache.commons.io.input.BOMInputStream;
3434

35-
import org.owasp.dependencycheck.utils.FileUtils;
35+
import org.owasp.dependencycheck.utils.AutoCloseableInputSource;
3636
import org.owasp.dependencycheck.utils.XmlUtils;
3737

3838
import org.slf4j.Logger;
3939
import org.slf4j.LoggerFactory;
40-
import org.xml.sax.EntityResolver;
4140
import org.xml.sax.InputSource;
4241
import org.xml.sax.SAXException;
4342
import org.xml.sax.XMLReader;
4443

44+
import static org.owasp.dependencycheck.utils.AutoCloseableInputSource.fromResource;
45+
4546
/**
4647
* A simple validating parser for XML Suppression Rules.
4748
*
@@ -105,24 +106,21 @@ public List<SuppressionRule> parseSuppressionRules(File file) throws Suppression
105106
*/
106107
public List<SuppressionRule> parseSuppressionRules(InputStream inputStream)
107108
throws SuppressionParseException, SAXException {
108-
try (
109-
InputStream schemaStream14 = FileUtils.getResourceAsStream(SUPPRESSION_SCHEMA_1_4);
110-
InputStream schemaStream13 = FileUtils.getResourceAsStream(SUPPRESSION_SCHEMA_1_3);
111-
InputStream schemaStream12 = FileUtils.getResourceAsStream(SUPPRESSION_SCHEMA_1_2);
112-
InputStream schemaStream11 = FileUtils.getResourceAsStream(SUPPRESSION_SCHEMA_1_1);
113-
InputStream schemaStream10 = FileUtils.getResourceAsStream(SUPPRESSION_SCHEMA_1_0)) {
109+
try (AutoCloseableInputSource schemaStream14 = fromResource(SUPPRESSION_SCHEMA_1_4);
110+
AutoCloseableInputSource schemaStream13 = fromResource(SUPPRESSION_SCHEMA_1_3);
111+
AutoCloseableInputSource schemaStream12 = fromResource(SUPPRESSION_SCHEMA_1_2);
112+
AutoCloseableInputSource schemaStream11 = fromResource(SUPPRESSION_SCHEMA_1_1);
113+
AutoCloseableInputSource schemaStream10 = fromResource(SUPPRESSION_SCHEMA_1_0)) {
114114

115115
final BOMInputStream bomStream = BOMInputStream.builder().setInputStream(inputStream).get();
116116
final ByteOrderMark bom = bomStream.getBOM();
117117
final String defaultEncoding = StandardCharsets.UTF_8.name();
118118
final String charsetName = bom == null ? defaultEncoding : bom.getCharsetName();
119119

120120
final SuppressionHandler handler = new SuppressionHandler();
121-
final SAXParser saxParser = XmlUtils.buildSecureSaxParser(schemaStream14, schemaStream13, schemaStream12, schemaStream11, schemaStream10);
122-
final XMLReader xmlReader = saxParser.getXMLReader();
121+
final XMLReader xmlReader = XmlUtils.buildSecureValidatingXmlReader(schemaStream14, schemaStream13, schemaStream12, schemaStream11, schemaStream10);
123122
xmlReader.setErrorHandler(new SuppressionErrorHandler());
124123
xmlReader.setContentHandler(handler);
125-
xmlReader.setEntityResolver(new ClassloaderResolver());
126124
try (Reader reader = new InputStreamReader(bomStream, charsetName)) {
127125
final InputSource in = new InputSource(reader);
128126
xmlReader.parse(in);
@@ -141,24 +139,4 @@ public List<SuppressionRule> parseSuppressionRules(InputStream inputStream)
141139
}
142140
}
143141

144-
/**
145-
* Load HTTPS schema resources locally from the JAR files resources.
146-
*/
147-
private static class ClassloaderResolver implements EntityResolver {
148-
149-
@Override
150-
public InputSource resolveEntity(String publicId, String systemId) throws SAXException, IOException {
151-
152-
if (systemId != null && systemId.startsWith("https://jeremylong.github.io/DependencyCheck/")) {
153-
final String resource = "schema/" + systemId.substring(45);
154-
final InputStream in = SuppressionParser.class.getClassLoader().getResourceAsStream(resource);
155-
if (in != null) {
156-
final InputSource source = new InputSource(in);
157-
source.setSystemId(systemId);
158-
return source;
159-
}
160-
}
161-
return null;
162-
}
163-
}
164142
}

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -858,9 +858,17 @@ public String toString() {
858858
return sb.toString();
859859
}
860860

861+
/**
862+
* Suppression rules are considered equal if all properties except the "notes" and mutual "matched"
863+
* status are equal.
864+
*
865+
* @param o the reference object with which to compare.
866+
* @return whether the object is equals to this one
867+
*/
861868
@Override
862869
public boolean equals(Object o) {
863870
if (o == null || getClass() != o.getClass()) return false;
871+
if (this == o) return true;
864872
SuppressionRule that = (SuppressionRule) o;
865873
return base == that.base
866874
&& Objects.equals(filePath, that.filePath)
@@ -875,12 +883,11 @@ public boolean equals(Object o) {
875883
&& Objects.equals(vulnerabilityNames, that.vulnerabilityNames)
876884
&& Objects.equals(gav, that.gav)
877885
&& Objects.equals(packageUrl, that.packageUrl)
878-
&& Objects.equals(notes, that.notes)
879886
&& Objects.equals(until, that.until);
880887
}
881888

882889
@Override
883890
public int hashCode() {
884-
return Objects.hash(filePath, sha1, cpe, cvssBelow, cvssV2Below, cvssV3Below, cvssV4Below, cwe, cve, vulnerabilityNames, gav, packageUrl, notes, base, until);
891+
return Objects.hash(base, filePath, sha1, cpe, cvssBelow, cvssV2Below, cvssV3Below, cvssV4Below, cwe, cve, vulnerabilityNames, gav, packageUrl, until);
885892
}
886893
}

‎core/src/test/java/org/owasp/dependencycheck/analyzer/AbstractSuppressionAnalyzerTest.java‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,16 @@ void testLoadCorePackagedSuppressions() throws Exception {
178178
assertAllHostedSnapshotSuppressionsAreMarkedAsBase(baseRules);
179179
}
180180

181+
private @NonNull List<SuppressionRule> assertAllBaseSuppressionRulesAreMarkedCorrectly() throws InvalidSettingException, InitializationException {
182+
getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, false);
183+
Engine engine = prepareSuppressions();
184+
185+
@SuppressWarnings("unchecked") List<SuppressionRule> baseRules = (List<SuppressionRule>) engine.getObject(SUPPRESSION_OBJECT_KEY);
186+
assertThat(baseRules, not(empty()));
187+
assertThat("Expected all suppressions in base file to be marked as base", allRulesNotMarkedAsBase(baseRules), empty());
188+
return baseRules;
189+
}
190+
181191
private void assertAllHostedSnapshotSuppressionsAreMarkedAsBase(List<SuppressionRule> baseRules) throws InvalidSettingException, InitializationException {
182192
getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, true);
183193
getSettings().setString(KEYS.HOSTED_SUPPRESSIONS_URL, "https://intentionally-bad-url/hosted-suppressions.xml");
@@ -190,16 +200,6 @@ private void assertAllHostedSnapshotSuppressionsAreMarkedAsBase(List<Suppression
190200
assertThat("Expected all suppressions in hosted suppressions snapshot file to be marked as base", allRulesNotMarkedAsBase(hostedSnapshotRules), empty());
191201
}
192202

193-
private @NonNull List<SuppressionRule> assertAllBaseSuppressionRulesAreMarkedCorrectly() throws InvalidSettingException, InitializationException {
194-
getSettings().setBoolean(KEYS.HOSTED_SUPPRESSIONS_ENABLED, false);
195-
Engine engine = prepareSuppressions();
196-
197-
@SuppressWarnings("unchecked") List<SuppressionRule> baseRules = (List<SuppressionRule>) engine.getObject(SUPPRESSION_OBJECT_KEY);
198-
assertThat(baseRules, not(empty()));
199-
assertThat("Expected all suppressions in base file to be marked as base", allRulesNotMarkedAsBase(baseRules), empty());
200-
return baseRules;
201-
}
202-
203203
private @NonNull List<SuppressionRule> allRulesNotMarkedAsBase(List<SuppressionRule> baseRules) {
204204
return baseRules.stream().filter(r -> !r.isBase()).collect(Collectors.toList());
205205
}

‎core/src/test/java/org/owasp/dependencycheck/xml/assembly/GrokHandlerTest.java‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@
1919

2020
import org.junit.jupiter.api.Test;
2121
import org.owasp.dependencycheck.BaseTest;
22+
import org.owasp.dependencycheck.utils.AutoCloseableInputSource;
2223
import org.owasp.dependencycheck.utils.XmlUtils;
2324
import org.xml.sax.InputSource;
2425
import org.xml.sax.XMLReader;
2526

26-
import javax.xml.parsers.SAXParser;
2727
import java.io.File;
2828
import java.io.FileInputStream;
2929
import java.io.InputStream;
@@ -32,6 +32,7 @@
3232
import java.nio.charset.StandardCharsets;
3333

3434
import static org.junit.jupiter.api.Assertions.assertEquals;
35+
import static org.owasp.dependencycheck.utils.AutoCloseableInputSource.fromResource;
3536

3637
/**
3738
*
@@ -47,24 +48,23 @@ class GrokHandlerTest extends BaseTest {
4748
@Test
4849
void testHandler() throws Exception {
4950
File file = BaseTest.getResourceAsFile(this, "assembly/sample-grok.xml");
50-
InputStream schemaStream = BaseTest.getResourceAsStream(this, "schema/grok-assembly.1.0.xsd");
5151

52-
GrokHandler handler = new GrokHandler();
53-
SAXParser saxParser = XmlUtils.buildSecureSaxParser(schemaStream);
54-
XMLReader xmlReader = saxParser.getXMLReader();
55-
xmlReader.setErrorHandler(new GrokErrorHandler());
56-
xmlReader.setContentHandler(handler);
52+
try (AutoCloseableInputSource schema = fromResource("schema/grok-assembly.1.0.xsd")) {
53+
GrokHandler handler = new GrokHandler();
54+
XMLReader xmlReader = XmlUtils.buildSecureValidatingXmlReader(schema);
55+
xmlReader.setErrorHandler(new GrokErrorHandler());
56+
xmlReader.setContentHandler(handler);
5757

58-
InputStream inputStream = new FileInputStream(file);
59-
Reader reader = new InputStreamReader(inputStream, StandardCharsets.UTF_8);
60-
InputSource in = new InputSource(reader);
58+
try (Reader reader = new InputStreamReader(new FileInputStream(file), StandardCharsets.UTF_8)) {
59+
InputSource in = new InputSource(reader);
60+
xmlReader.parse(in);
61+
}
6162

62-
xmlReader.parse(in);
63-
64-
AssemblyData result = handler.getAssemblyData();
65-
assertEquals("OWASP Contributors", result.getCompanyName());
66-
assertEquals("GrokAssembly", result.getProductName());
67-
assertEquals("Inspects a .NET Assembly to determine Company, Product, and Version information", result.getComments());
68-
assertEquals(1, result.getNamespaces().size());
63+
AssemblyData result = handler.getAssemblyData();
64+
assertEquals("OWASP Contributors", result.getCompanyName());
65+
assertEquals("GrokAssembly", result.getProductName());
66+
assertEquals("Inspects a .NET Assembly to determine Company, Product, and Version information", result.getComments());
67+
assertEquals(1, result.getNamespaces().size());
68+
}
6969
}
7070
}

0 commit comments

Comments
 (0)