Skip to content

Commit c82a59c

Browse files
chadlwilsonchadlwilson
authored
fix(core): correct xml schema validation handling without needing external access (#8272)
Signed-off-by: Chad Wilson <29788154+chadlwilson@users.noreply.github.com>
1 parent 1a2bfc0 commit c82a59c

33 files changed

Lines changed: 694 additions & 342 deletions

‎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: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,9 @@ private void loadHostedSuppressionBaseData(final SuppressionParser parser, final
263263
HostedSuppressionsDataSource.DEFAULT_SUPPRESSIONS_URL);
264264
final URL url = new URL(configuredUrl);
265265
final String fileName = new File(url.getPath()).getName();
266+
if (fileName.isBlank()) {
267+
throw new IOException("Hosted Suppression URL must imply a filename");
268+
}
266269
final File repoFile = new File(getSettings().getDataDirectory(), fileName);
267270
boolean repoEmpty = !repoFile.isFile() || repoFile.length() <= 1L;
268271
if (repoEmpty) {
@@ -333,18 +336,6 @@ private void loadCachedHostedSuppressionsRules(final SuppressionParser parser, f
333336
}
334337
}
335338

336-
private static boolean forceUpdateHostedSuppressions(final Engine engine, final File repoFile) {
337-
final HostedSuppressionsDataSource ds = new HostedSuppressionsDataSource();
338-
boolean repoEmpty = true;
339-
try {
340-
ds.update(engine);
341-
repoEmpty = !repoFile.isFile() || repoFile.length() <= 1L;
342-
} catch (UpdateException ex) {
343-
LOGGER.warn("Failed to update the Hosted Suppression file", ex);
344-
}
345-
return repoEmpty;
346-
}
347-
348339
/**
349340
* Load a single suppression rules file from the path provided using the
350341
* parser provided.
@@ -407,19 +398,17 @@ private List<SuppressionRule> loadSuppressionFile(final SuppressionParser parser
407398
}
408399
}
409400
}
410-
if (file != null) {
411-
if (!file.exists()) {
412-
final String msg = String.format("Suppression file '%s' does not exist", file.getPath());
413-
LOGGER.warn(msg);
414-
throw new SuppressionParseException(msg);
415-
}
416-
try {
417-
list.addAll(parser.parseSuppressionRules(file));
418-
} catch (SuppressionParseException ex) {
419-
LOGGER.warn("Unable to parse suppression xml file '{}'", file.getPath());
420-
LOGGER.warn(ex.getMessage());
421-
throw ex;
422-
}
401+
if (!file.exists()) {
402+
final String msg = String.format("Suppression file '%s' does not exist", file.getPath());
403+
LOGGER.warn(msg);
404+
throw new SuppressionParseException(msg);
405+
}
406+
try {
407+
list.addAll(parser.parseSuppressionRules(file));
408+
} catch (SuppressionParseException ex) {
409+
LOGGER.warn("Unable to parse suppression xml file '{}'", file.getPath());
410+
LOGGER.warn(ex.getMessage());
411+
throw ex;
423412
}
424413
} catch (DownloadFailedException ex) {
425414
throwSuppressionParseException("Unable to fetch the configured suppression file", ex, suppressionFilePath);

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

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -407,9 +407,6 @@ public void prepareFileTypeAnalyzer(Engine engine) throws InitializationExceptio
407407
private File extractGrokAssembly() throws InitializationException {
408408
final File location;
409409
try (InputStream in = FileUtils.getResourceAsStream("GrokAssembly.zip")) {
410-
if (in == null) {
411-
throw new InitializationException("Unable to extract GrokAssembly.dll - file not found");
412-
}
413410
location = FileUtils.createTempDirectory(getSettings().getTempDirectory());
414411
ExtractionUtil.extractFiles(in, location);
415412
} catch (ExtractionException ex) {

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

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
*/
1818
package org.owasp.dependencycheck.analyzer;
1919

20+
import org.jetbrains.annotations.VisibleForTesting;
2021
import org.owasp.dependencycheck.Engine;
2122
import org.owasp.dependencycheck.analyzer.exception.AnalysisException;
2223
import org.owasp.dependencycheck.dependency.Dependency;
@@ -248,9 +249,6 @@ private void loadHintRules() throws HintParseException {
248249
final HintParser parser = new HintParser();
249250
File file = null;
250251
try (InputStream in = FileUtils.getResourceAsStream(HINT_RULE_FILE_NAME)) {
251-
if (in == null) {
252-
throw new HintParseException("Hint rules `" + HINT_RULE_FILE_NAME + "` could not be found");
253-
}
254252
parser.parseHints(in);
255253
} catch (SAXException | IOException ex) {
256254
throw new HintParseException("Error parsing hints: " + ex.getMessage(), ex);
@@ -299,23 +297,19 @@ private void loadHintRules() throws HintParseException {
299297
}
300298
}
301299

302-
if (file == null) {
303-
throw new HintParseException("Unable to locate hints file:" + filePath);
304-
} else {
305-
try {
306-
parser.parseHints(file);
307-
if (parser.getHintRules() != null && !parser.getHintRules().isEmpty()) {
308-
localHints.addAll(parser.getHintRules());
309-
}
310-
if (parser.getVendorDuplicatingHintRules() != null && !parser.getVendorDuplicatingHintRules().isEmpty()) {
311-
localVendorHints.addAll(parser.getVendorDuplicatingHintRules());
312-
}
313-
} catch (HintParseException ex) {
314-
LOGGER.warn("Unable to parse hint rule xml file '{}'", file.getPath());
315-
LOGGER.warn(ex.getMessage());
316-
LOGGER.debug("", ex);
317-
throw ex;
300+
try {
301+
parser.parseHints(file);
302+
if (parser.getHintRules() != null && !parser.getHintRules().isEmpty()) {
303+
localHints.addAll(parser.getHintRules());
318304
}
305+
if (parser.getVendorDuplicatingHintRules() != null && !parser.getVendorDuplicatingHintRules().isEmpty()) {
306+
localVendorHints.addAll(parser.getVendorDuplicatingHintRules());
307+
}
308+
} catch (HintParseException ex) {
309+
LOGGER.warn("Unable to parse hint rule xml file '{}'", file.getPath());
310+
LOGGER.warn(ex.getMessage());
311+
LOGGER.debug("", ex);
312+
throw ex;
319313
}
320314
} catch (DownloadFailedException ex) {
321315
throw new HintParseException("Unable to fetch the configured hint file", ex);
@@ -334,4 +328,14 @@ private void loadHintRules() throws HintParseException {
334328
LOGGER.debug("{} hint rules were loaded.", hints.length);
335329
LOGGER.debug("{} duplicating hint rules were loaded.", vendorHints.length);
336330
}
331+
332+
@VisibleForTesting
333+
HintRule[] getHintRules() {
334+
return hints;
335+
}
336+
337+
@VisibleForTesting
338+
VendorDuplicatingHintRule[] getVendorDuplicatingHintRules() {
339+
return vendorHints;
340+
}
337341
}

‎core/src/main/java/org/owasp/dependencycheck/data/cache/DataCacheFactory.java‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,6 @@ public DataCacheFactory(Settings settings) {
104104
throw new CacheException("Unable to create disk cache: " + cacheDirectory);
105105
}
106106
try (InputStream in = FileUtils.getResourceAsStream(CACHE_PROPERTIES)) {
107-
if (in == null) {
108-
throw new RuntimeException("Cache properties `" + CACHE_PROPERTIES + "` could not be found");
109-
}
110-
111107
final Properties properties = new Properties();
112108
properties.load(in);
113109
properties.put("jcs.auxiliary.ODC.attributes.DiskPath", cacheDirectory.getCanonicalPath());

‎core/src/main/java/org/owasp/dependencycheck/reporting/ReportGenerator.java‎

Lines changed: 18 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import org.slf4j.LoggerFactory;
4242
import org.xml.sax.InputSource;
4343
import org.xml.sax.SAXException;
44-
import org.xml.sax.XMLReader;
4544

4645
import javax.annotation.concurrent.NotThreadSafe;
4746
import javax.xml.XMLConstants;
@@ -445,48 +444,31 @@ protected void processTemplate(String template, File file) throws ReportExceptio
445444
* @throws ReportException is thrown when an exception occurs
446445
*/
447446
protected void processTemplate(String templateName, OutputStream outputStream) throws ReportException {
448-
InputStream input = null;
449-
String logTag;
450-
final File f = new File(templateName);
451447
try {
448+
String logTag;
449+
InputStream input;
450+
final File f = new File(templateName);
452451
if (f.isFile()) {
453-
try {
454-
logTag = templateName;
455-
input = new FileInputStream(f);
456-
} catch (FileNotFoundException ex) {
457-
throw new ReportException("Unable to locate template file: " + templateName, ex);
458-
}
452+
logTag = templateName;
453+
input = new FileInputStream(f);
459454
} else {
460455
logTag = "templates/" + templateName + ".vsl";
461456
input = FileUtils.getResourceAsStream(logTag);
462457
}
463-
if (input == null) {
464-
logTag = templateName;
465-
input = FileUtils.getResourceAsStream(templateName);
466-
}
467-
if (input == null) {
468-
throw new ReportException("Template file doesn't exist: " + logTag);
469-
}
470458

471459
try (InputStreamReader reader = new InputStreamReader(input, StandardCharsets.UTF_8);
472460
OutputStreamWriter writer = new OutputStreamWriter(outputStream, StandardCharsets.UTF_8)) {
473461
if (!velocityEngine.evaluate(context, writer, logTag, reader)) {
474462
throw new ReportException("Failed to convert the template into html.");
475463
}
476464
writer.flush();
477-
} catch (UnsupportedEncodingException ex) {
478-
throw new ReportException("Unable to generate the report using UTF-8", ex);
479465
}
466+
} catch (UnsupportedEncodingException ex) {
467+
throw new ReportException("Unable to generate the report using UTF-8", ex);
468+
} catch (FileNotFoundException ex) {
469+
throw new ReportException("Unable to locate template file: " + templateName, ex);
480470
} catch (IOException ex) {
481471
throw new ReportException("Unable to write the report", ex);
482-
} finally {
483-
if (input != null) {
484-
try {
485-
input.close();
486-
} catch (IOException ex) {
487-
LOGGER.trace("Error closing input", ex);
488-
}
489-
}
490472
}
491473
}
492474

@@ -513,9 +495,8 @@ private void ensureParentDirectoryExists(File file) throws ReportException {
513495
* Reformats the given XML file.
514496
*
515497
* @param path the path to the XML file to be reformatted
516-
* @throws ReportException thrown if the given JSON file is malformed
517498
*/
518-
private void pretifyXml(String path) throws ReportException {
499+
private void pretifyXml(String path) {
519500
final String outputPath = path + ".pretty";
520501
final File in = new File(path);
521502
final File out = new File(outputPath);
@@ -527,10 +508,7 @@ private void pretifyXml(String path) throws ReportException {
527508
transformer.setOutputProperty(OutputKeys.INDENT, "yes");
528509
transformer.setOutputProperty("{http://xml.apache.org/xslt}indent-amount", "2");
529510

530-
final SAXSource saxs = new SAXSource(new InputSource(path));
531-
final XMLReader saxReader = XmlUtils.buildSecureSaxParser().getXMLReader();
532-
533-
saxs.setXMLReader(saxReader);
511+
final SAXSource saxs = new SAXSource(XmlUtils.buildSecureXmlReader(), new InputSource(path));
534512
transformer.transform(saxs, new StreamResult(new OutputStreamWriter(os, StandardCharsets.UTF_8)));
535513
} catch (ParserConfigurationException | TransformerConfigurationException ex) {
536514
LOGGER.debug("Configuration exception when pretty printing", ex);
@@ -539,17 +517,7 @@ private void pretifyXml(String path) throws ReportException {
539517
LOGGER.debug("Malformed XML?", ex);
540518
LOGGER.error("Unable to generate pretty report, caused by: {}", ex.getMessage());
541519
}
542-
if (out.isFile() && in.isFile() && in.delete()) {
543-
try {
544-
Thread.sleep(1000);
545-
Files.move(out.toPath(), in.toPath());
546-
} catch (IOException ex) {
547-
LOGGER.error("Unable to generate pretty report, caused by: {}", ex.getMessage());
548-
} catch (InterruptedException ex) {
549-
Thread.currentThread().interrupt();
550-
LOGGER.error("Unable to generate pretty report, caused by: {}", ex.getMessage());
551-
}
552-
}
520+
replaceWithPrettified(out, in);
553521
}
554522

555523
/**
@@ -581,10 +549,14 @@ private void pretifyJson(String pathToJson) throws ReportException {
581549
LOGGER.debug("Malformed JSON?", ex);
582550
throw new ReportException("Unable to generate json report", ex);
583551
}
584-
if (out.isFile() && in.isFile() && in.delete()) {
552+
replaceWithPrettified(out, in);
553+
}
554+
555+
private void replaceWithPrettified(File prettified, File original) {
556+
if (prettified.isFile() && original.isFile() && original.delete()) {
585557
try {
586558
Thread.sleep(1000);
587-
Files.move(out.toPath(), in.toPath());
559+
Files.move(prettified.toPath(), original.toPath());
588560
} catch (IOException ex) {
589561
LOGGER.error("Unable to generate pretty report, caused by: {}", ex.getMessage());
590562
} catch (InterruptedException ex) {

‎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)) {

0 commit comments

Comments
 (0)