From e578b482ef61c9a65d3f59e9fdea8b7c22ee3048 Mon Sep 17 00:00:00 2001 From: knalli Date: Mon, 18 Dec 2023 21:13:12 +0100 Subject: [PATCH 1/2] fix: ensure open handles will not leak on errors --- .../nvd/api/CveApiJson20CveItemSource.java | 17 +++------ .../nvd/api/JsonArrayCveItemSource.java | 17 +++------ .../data/update/nvd/api/NvdApiProcessor.java | 36 ++++++++++++------- 3 files changed, 32 insertions(+), 38 deletions(-) diff --git a/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/CveApiJson20CveItemSource.java b/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/CveApiJson20CveItemSource.java index c9322d18248..26b7bd19b7e 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/CveApiJson20CveItemSource.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/CveApiJson20CveItemSource.java @@ -22,30 +22,23 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import io.github.jeremylong.openvulnerability.client.nvd.DefCveItem; +import org.apache.commons.io.IOUtils; -import java.io.BufferedInputStream; -import java.io.File; import java.io.IOException; import java.io.InputStream; -import java.nio.file.Files; -import java.util.zip.GZIPInputStream; public class CveApiJson20CveItemSource implements CveItemSource { - private final File jsonFile; private final ObjectMapper mapper; private final InputStream inputStream; private final JsonParser jsonParser; private DefCveItem currentItem; private DefCveItem nextItem; - public CveApiJson20CveItemSource(File jsonFile) throws IOException { - this.jsonFile = jsonFile; + public CveApiJson20CveItemSource(InputStream inputStream) throws IOException { mapper = new ObjectMapper(); mapper.registerModule(new JavaTimeModule()); - inputStream = jsonFile.getName().endsWith(".gz") ? - new BufferedInputStream(new GZIPInputStream(Files.newInputStream(jsonFile.toPath()))) : - new BufferedInputStream(Files.newInputStream(jsonFile.toPath())); + this.inputStream = inputStream; jsonParser = mapper.getFactory().createParser(inputStream); JsonToken token = null; @@ -62,9 +55,7 @@ public CveApiJson20CveItemSource(File jsonFile) throws IOException { @Override public void close() throws Exception { - jsonParser.close(); - inputStream.close(); - Files.delete(jsonFile.toPath()); + IOUtils.closeQuietly(jsonParser, inputStream); } @Override diff --git a/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/JsonArrayCveItemSource.java b/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/JsonArrayCveItemSource.java index cd82358e6e1..e3928b3e629 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/JsonArrayCveItemSource.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/JsonArrayCveItemSource.java @@ -22,30 +22,23 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import io.github.jeremylong.openvulnerability.client.nvd.DefCveItem; +import org.apache.commons.io.IOUtils; -import java.io.BufferedInputStream; -import java.io.File; import java.io.IOException; import java.io.InputStream; -import java.nio.file.Files; -import java.util.zip.GZIPInputStream; public class JsonArrayCveItemSource implements CveItemSource { - private final File jsonFile; private final ObjectMapper mapper; private final InputStream inputStream; private final JsonParser jsonParser; private DefCveItem currentItem; private DefCveItem nextItem; - public JsonArrayCveItemSource(File jsonFile) throws IOException { - this.jsonFile = jsonFile; + public JsonArrayCveItemSource(InputStream inputStream) throws IOException { mapper = new ObjectMapper(); mapper.registerModule(new JavaTimeModule()); - inputStream = jsonFile.getName().endsWith(".gz") ? - new BufferedInputStream(new GZIPInputStream(Files.newInputStream(jsonFile.toPath()))) : - new BufferedInputStream(Files.newInputStream(jsonFile.toPath())); + this.inputStream = inputStream; jsonParser = mapper.getFactory().createParser(inputStream); if (jsonParser.nextToken() == JsonToken.START_ARRAY) { @@ -55,9 +48,7 @@ public JsonArrayCveItemSource(File jsonFile) throws IOException { @Override public void close() throws Exception { - jsonParser.close(); - inputStream.close(); - Files.delete(jsonFile.toPath()); + IOUtils.closeQuietly(jsonParser, inputStream); } @Override diff --git a/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/NvdApiProcessor.java b/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/NvdApiProcessor.java index dd41664fd05..b01da9c3b69 100644 --- a/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/NvdApiProcessor.java +++ b/core/src/main/java/org/owasp/dependencycheck/data/update/nvd/api/NvdApiProcessor.java @@ -18,8 +18,15 @@ package org.owasp.dependencycheck.data.update.nvd.api; import io.github.jeremylong.openvulnerability.client.nvd.DefCveItem; + +import java.io.BufferedInputStream; import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; import java.util.concurrent.Callable; +import java.util.zip.GZIPInputStream; + import org.owasp.dependencycheck.data.nvd.ecosystem.CveEcosystemMapper; import org.owasp.dependencycheck.data.nvdcve.CveDB; import org.slf4j.Logger; @@ -82,16 +89,7 @@ public NvdApiProcessor(final CveDB cveDB, File jsonFile) { @Override public NvdApiProcessor call() throws Exception { - CveItemSource itemSource = null; - - if (jsonFile.getName().endsWith(".jsonarray.gz")) { - itemSource = new JsonArrayCveItemSource(jsonFile); - } else if (jsonFile.getName().endsWith(".gz")) { - itemSource = new CveApiJson20CveItemSource(jsonFile); - } else { - itemSource = new JsonArrayCveItemSource(jsonFile); - } - try { + try (CveItemSource itemSource = buildItemSource(jsonFile)) { while (itemSource.hasNext()) { DefCveItem entry = itemSource.next(); try { @@ -100,13 +98,27 @@ public NvdApiProcessor call() throws Exception { LOGGER.error("Failed to process " + entry.getCve().getId(), ex); } } - } finally { - itemSource.close(); } endTime = System.currentTimeMillis(); return this; } + static CveItemSource buildItemSource(File file) throws IOException { + if (file.getName().endsWith(".jsonarray.gz")) { + return new JsonArrayCveItemSource(new BufferedInputStream(new GZIPInputStream( + Files.newInputStream(file.toPath()) + ))); + } else if (file.getName().endsWith(".gz")) { + return new CveApiJson20CveItemSource(new BufferedInputStream(new GZIPInputStream( + Files.newInputStream(file.toPath()) + ))); + } else { + return new JsonArrayCveItemSource(new BufferedInputStream( + Files.newInputStream(file.toPath()) + )); + } + } + /** * Calculates how long the update process took. * From b55ca3c7e31edb5f4e9c335f47d62f2f5c0cfa3f Mon Sep 17 00:00:00 2001 From: knalli Date: Mon, 18 Dec 2023 23:47:01 +0100 Subject: [PATCH 2/2] feat: add unit test for NvdApiProcessor --- .../update/nvd/api/NvdApiProcessorTest.java | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 core/src/test/java/org/owasp/dependencycheck/data/update/nvd/api/NvdApiProcessorTest.java diff --git a/core/src/test/java/org/owasp/dependencycheck/data/update/nvd/api/NvdApiProcessorTest.java b/core/src/test/java/org/owasp/dependencycheck/data/update/nvd/api/NvdApiProcessorTest.java new file mode 100644 index 00000000000..60e474ad683 --- /dev/null +++ b/core/src/test/java/org/owasp/dependencycheck/data/update/nvd/api/NvdApiProcessorTest.java @@ -0,0 +1,83 @@ +/* + * Copyright 2014 OWASP. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.owasp.dependencycheck.data.update.nvd.api; + +import com.fasterxml.jackson.core.JsonParseException; +import org.junit.Test; +import org.owasp.dependencycheck.BaseTest; +import org.owasp.dependencycheck.data.nvdcve.CveDB; +import org.owasp.dependencycheck.utils.Settings; + +import java.io.File; +import java.io.FileWriter; +import java.io.IOException; +import java.nio.file.NoSuchFileException; + +import static org.junit.Assert.assertThrows; + +/** + * + * @author Jeremy Long + */ +public class NvdApiProcessorTest extends BaseTest { + + @Test + public void doesNotExistFile() throws Exception { + try (CveDB cve = new CveDB(getSettings())) { + File file = new File("does_not_exist"); + NvdApiProcessor processor = new NvdApiProcessor(null, file); + assertThrows(NoSuchFileException.class, processor::call); + } + } + + @Test + public void unspecifiedFileName() throws Exception { + try (CveDB cve = new CveDB(getSettings())) { + File file = File.createTempFile("test", "test"); + writeFileString(file, ""); + NvdApiProcessor processor = new NvdApiProcessor(null, file); + processor.call(); + } + } + + @Test + public void invalidFileContent() throws Exception { + try (CveDB cve = new CveDB(getSettings())) { + File file = File.createTempFile("test", "test.json"); + // invalid content (broken array) + writeFileString(file, "[}"); + NvdApiProcessor processor = new NvdApiProcessor(null, file); + assertThrows(JsonParseException.class, processor::call); + } + } + + @Test + public void processValidStructure() throws Exception { + try (CveDB cve = new CveDB(getSettings())) { + File file = File.createTempFile("test", "test.json"); + writeFileString(file, "[]"); + NvdApiProcessor processor = new NvdApiProcessor(null, file); + processor.call(); + } + } + + static void writeFileString(File file, String content) throws IOException { + try (FileWriter writer = new FileWriter(file, false)) { + writer.write(content); + writer.flush(); + } + } +}