From 725f1e975920e596b5fec6f435fa48d9dcd83f0f Mon Sep 17 00:00:00 2001 From: vladt Date: Tue, 4 Jul 2017 16:45:12 -0400 Subject: [PATCH 1/5] Fixed the creation of the GrokAssembly.exe temp file and the cleanup of the temp config file. --- .../analyzer/AssemblyAnalyzer.java | 27 +++++++--- .../analyzer/AssemblyAnalyzerTest.java | 51 ++++++++++++++++++- 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java index be81cac89..a977d76c6 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java @@ -73,6 +73,10 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { * The temp value for GrokAssembly.exe */ private File grokAssemblyExe = null; + /** + * The temp value for GrokAssembly.exe.config + */ + private File grokAssemblyConfig = null; /** * Logger */ @@ -201,22 +205,24 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { @Override public void initializeFileTypeAnalyzer() throws InitializationException { final File tempFile; - final String cfg; + final File cfgFile; try { tempFile = File.createTempFile("GKA", ".exe", Settings.getTempDirectory()); - cfg = tempFile.getPath() + ".config"; + cfgFile = new File(tempFile.getPath() + ".config"); } catch (IOException ex) { setEnabled(false); throw new InitializationException("Unable to create temporary file for the assembly analyzer", ex); } try (FileOutputStream fos = new FileOutputStream(tempFile); - InputStream is = FileUtils.getResourceAsStream("GrokAssembly.exe"); - FileOutputStream fosCfg = new FileOutputStream(cfg); - InputStream isCfg = FileUtils.getResourceAsStream("GrokAssembly.exe.config")) { + InputStream is = FileUtils.getResourceAsStream("GrokAssembly.exe"); + FileOutputStream fosCfg = new FileOutputStream(cfgFile); + InputStream isCfg = FileUtils.getResourceAsStream("GrokAssembly.exe.config")) { + IOUtils.copy(is, fos); grokAssemblyExe = tempFile; LOGGER.debug("Extracted GrokAssembly.exe to {}", grokAssemblyExe.getPath()); IOUtils.copy(isCfg, fosCfg); - LOGGER.debug("Extracted GrokAssembly.exe.config to {}", cfg); + grokAssemblyConfig = cfgFile; + LOGGER.debug("Extracted GrokAssembly.exe.config to {}", cfgFile); } catch (IOException ioe) { this.setEnabled(false); LOGGER.warn("Could not extract GrokAssembly.exe: {}", ioe.getMessage()); @@ -287,6 +293,15 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { LOGGER.debug("Can't delete temporary GrokAssembly.exe"); grokAssemblyExe.deleteOnExit(); } + try { + if (grokAssemblyConfig != null && !grokAssemblyConfig.delete()) { + LOGGER.debug("Unable to delete temporary GrokAssembly.exe.config; attempting delete on exit"); + grokAssemblyConfig.deleteOnExit(); + } + } catch (SecurityException se) { + LOGGER.debug("Can't delete temporary GrokAssembly.exe.config"); + grokAssemblyConfig.deleteOnExit(); + } } /** diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java index b1d02f51c..a9c045ea2 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java @@ -18,8 +18,16 @@ package org.owasp.dependencycheck.analyzer; import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; + +import org.apache.commons.io.IOUtils; import org.junit.After; + +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import org.junit.Assume; @@ -33,6 +41,7 @@ import org.owasp.dependencycheck.dependency.Confidence; import org.owasp.dependencycheck.dependency.Dependency; import org.owasp.dependencycheck.dependency.Evidence; import org.owasp.dependencycheck.exception.InitializationException; +import org.owasp.dependencycheck.utils.FileUtils; import org.owasp.dependencycheck.utils.Settings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -51,6 +60,10 @@ public class AssemblyAnalyzerTest extends BaseTest { private AssemblyAnalyzer analyzer; + private File grokAssemblyExeFile; + + private File grokAssemblyConfigFile; + /** * Sets up the analyzer. * @@ -62,7 +75,7 @@ public class AssemblyAnalyzerTest extends BaseTest { analyzer = new AssemblyAnalyzer(); analyzer.accept(new File("test.dll")); // trick into "thinking it is active" analyzer.initialize(); - Assume.assumeTrue("Mono is not installed, skipping tests.", analyzer.buildArgumentList() == null); + assertGrokAssembly(); } catch (Exception e) { if (e.getMessage().contains("Could not execute .NET AssemblyAnalyzer")) { LOGGER.warn("Exception setting up AssemblyAnalyzer. Tests will be incomplete"); @@ -73,6 +86,36 @@ public class AssemblyAnalyzerTest extends BaseTest { } } + private void assertGrokAssembly() throws IOException { + // There must be an .exe and a .config files created in the temp + // directory and they must match the resources they were created from. + File tempDirectory = Settings.getTempDirectory(); + for (File file : tempDirectory.listFiles()) { + String filename = file.getName(); + if (filename.startsWith("GKA") && filename.endsWith(".exe")) { + grokAssemblyExeFile = file; + break; + } + } + assertTrue("The GrokAssembly executable was not created.", grokAssemblyExeFile.isFile()); + grokAssemblyConfigFile = new File(grokAssemblyExeFile.getPath() + ".config"); + assertTrue("The GrokAssembly config was not created.", grokAssemblyConfigFile.isFile()); + + assertFileContent("The GrokAssembly executable has incorrect content.", "GrokAssembly.exe", + grokAssemblyExeFile); + assertFileContent("The GrokAssembly config has incorrect content.", "GrokAssembly.exe.config", + grokAssemblyConfigFile); + } + + private void assertFileContent(String message, String expectedResourceName, File actualFile) throws IOException { + try (InputStream expectedStream = FileUtils.getResourceAsStream(expectedResourceName); + InputStream actualStream = new FileInputStream(actualFile)) { + byte[] expectedBytes = IOUtils.toByteArray(expectedStream); + byte[] actualBytes = IOUtils.toByteArray(actualStream); + assertArrayEquals(message, expectedBytes, actualBytes); + } + } + /** * Tests to make sure the name is correct. */ @@ -180,5 +223,11 @@ public class AssemblyAnalyzerTest extends BaseTest { @After public void tearDown() throws Exception { analyzer.close(); + if (grokAssemblyExeFile != null) { + assertFalse(grokAssemblyExeFile.exists()); + } + if (grokAssemblyConfigFile != null) { + assertFalse(grokAssemblyConfigFile.exists()); + } } } From d5503ff6150ab11fc3462e2236b476d0e6b59c78 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Thu, 6 Jul 2017 06:05:26 -0400 Subject: [PATCH 2/5] updated error reporting for non-existent files --- .../owasp/dependencycheck/analyzer/AssemblyAnalyzer.java | 7 +++++++ .../dependencycheck/analyzer/AssemblyAnalyzerTest.java | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java index a977d76c6..4f57dd6a7 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzer.java @@ -113,6 +113,13 @@ public class AssemblyAnalyzer extends AbstractFileTypeAnalyzer { @Override public void analyzeDependency(Dependency dependency, Engine engine) throws AnalysisException { + + File test = new File(dependency.getActualFilePath()); + if (!test.isFile()) { + throw new AnalysisException(String.format("%s does not exist and cannot be analyzed by dependency-check", + dependency.getActualFilePath())); + } + if (grokAssemblyExe == null) { LOGGER.warn("GrokAssembly didn't get deployed"); return; diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java index a9c045ea2..d3e67b3fc 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java @@ -173,7 +173,7 @@ public class AssemblyAnalyzerTest extends BaseTest { analyzer.analyze(d, null); fail("Expected an AnalysisException"); } catch (AnalysisException ae) { - assertEquals("File does not exist", ae.getMessage()); + assertTrue(ae.getMessage().contains("nonexistent.dll does not exist and cannot be analyzed by dependency-check")); } finally { System.setProperty(LOG_KEY, oldProp); } From d76832f7619324b84209cc1add603b9cff60d006 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Thu, 6 Jul 2017 06:17:49 -0400 Subject: [PATCH 3/5] updated tear down to call the correct close method for this test --- .../owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java index d3e67b3fc..dc591262f 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java @@ -222,7 +222,7 @@ public class AssemblyAnalyzerTest extends BaseTest { @After public void tearDown() throws Exception { - analyzer.close(); + analyzer.closeAnalyzer(); if (grokAssemblyExeFile != null) { assertFalse(grokAssemblyExeFile.exists()); } From 3ffb2d1312456d231c2f604f9cf328e53f87a517 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Thu, 6 Jul 2017 06:31:20 -0400 Subject: [PATCH 4/5] removed un-needed checks in tearDown --- .../dependencycheck/analyzer/AssemblyAnalyzerTest.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java index dc591262f..25dfeb107 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java @@ -223,11 +223,5 @@ public class AssemblyAnalyzerTest extends BaseTest { @After public void tearDown() throws Exception { analyzer.closeAnalyzer(); - if (grokAssemblyExeFile != null) { - assertFalse(grokAssemblyExeFile.exists()); - } - if (grokAssemblyConfigFile != null) { - assertFalse(grokAssemblyConfigFile.exists()); - } } } From eb244e0234f16344fdb8818bd431aa35971c910c Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Thu, 6 Jul 2017 06:55:16 -0400 Subject: [PATCH 5/5] minor code quality cleanup per codacy --- .../dependencycheck/analyzer/AssemblyAnalyzerTest.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java index 25dfeb107..1ff39bc60 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/AssemblyAnalyzerTest.java @@ -27,7 +27,6 @@ import org.junit.After; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import org.junit.Assume; @@ -60,10 +59,6 @@ public class AssemblyAnalyzerTest extends BaseTest { private AssemblyAnalyzer analyzer; - private File grokAssemblyExeFile; - - private File grokAssemblyConfigFile; - /** * Sets up the analyzer. * @@ -89,6 +84,9 @@ public class AssemblyAnalyzerTest extends BaseTest { private void assertGrokAssembly() throws IOException { // There must be an .exe and a .config files created in the temp // directory and they must match the resources they were created from. + File grokAssemblyExeFile = null; + File grokAssemblyConfigFile = null; + File tempDirectory = Settings.getTempDirectory(); for (File file : tempDirectory.listFiles()) { String filename = file.getName();