From 8c6b9f9c6818f2837ec66c4aa82fc4c6d71844d2 Mon Sep 17 00:00:00 2001 From: David Jahn Date: Mon, 25 Apr 2016 09:40:54 -0400 Subject: [PATCH 1/3] Fixed CVSS for Ruby. this bug was discovered when scanning ruby applications and getting back `-1` cvss. this turns out to be a problem with bundle-audit cve database. Our solution was to use the NVD database, which dependency check uses to get the CVSS scores for Ruby only if the Criticality is missing from bundle-audit output. Keep in mind there are compilation errors with the commit atm. Fixes #485 Signed-off-by: Gabriel Ramirez --- .../analyzer/RubyBundleAuditAnalyzer.java | 6 ++++ .../dependencycheck/data/nvdcve/CveDB.java | 2 +- .../analyzer/RubyBundleAuditAnalyzerTest.java | 34 +++++++++++++++++-- .../data/nvdcve/CveDBIntegrationTest.java | 20 +++++++++++ 4 files changed, 59 insertions(+), 3 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java index a78838c11..55ad6a405 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java @@ -20,6 +20,7 @@ package org.owasp.dependencycheck.analyzer; import org.apache.commons.io.FileUtils; import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.analyzer.exception.AnalysisException; +import org.owasp.dependencycheck.data.nvdcve.CveDB; import org.owasp.dependencycheck.dependency.Confidence; import org.owasp.dependencycheck.dependency.Dependency; import org.owasp.dependencycheck.dependency.Reference; @@ -58,6 +59,10 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { public static final String ADVISORY = "Advisory: "; public static final String CRITICALITY = "Criticality: "; + public static CveDB CVEDB = new CveDB(); + //instance.open(); + //Vulnerability result = instance.getVulnerability("CVE-2015-3225"); + /** * @return a filter that accepts files named Gemfile.lock */ @@ -300,6 +305,7 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { } else if ("Low".equals(criticality)) { vulnerability.setCvssScore(2.0f); } else { + //vulnerability.getName() vulnerability.setCvssScore(-1.0f); } } diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java index 42bceef0d..037da7564 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/data/nvdcve/CveDB.java @@ -372,7 +372,7 @@ public class CveDB { * @return a vulnerability object * @throws DatabaseException if an exception occurs */ - private Vulnerability getVulnerability(String cve) throws DatabaseException { + public Vulnerability getVulnerability(String cve) throws DatabaseException { PreparedStatement psV = null; PreparedStatement psR = null; PreparedStatement psS = null; diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzerTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzerTest.java index 8ef16ac40..68436e92e 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzerTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzerTest.java @@ -18,6 +18,7 @@ package org.owasp.dependencycheck.analyzer; import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -32,6 +33,7 @@ import org.owasp.dependencycheck.Engine; import org.owasp.dependencycheck.analyzer.exception.AnalysisException; import org.owasp.dependencycheck.data.nvdcve.DatabaseException; import org.owasp.dependencycheck.dependency.Dependency; +import org.owasp.dependencycheck.dependency.Vulnerability; import org.owasp.dependencycheck.utils.Settings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -106,17 +108,43 @@ public class RubyBundleAuditAnalyzerTest extends BaseTest { analyzer.analyze(result, engine); int size = engine.getDependencies().size(); assertThat(size, is(1)); - + Dependency dependency = engine.getDependencies().get(0); assertTrue(dependency.getProductEvidence().toString().toLowerCase().contains("redcarpet")); assertTrue(dependency.getVersionEvidence().toString().toLowerCase().contains("2.2.2")); - + + } catch (Exception e) { LOGGER.warn("Exception setting up RubyBundleAuditAnalyzer. Make sure Ruby gem bundle-audit is installed. You may also need to set property \"analyzer.bundle.audit.path\".", e); Assume.assumeNoException("Exception setting up RubyBundleAuditAnalyzer; bundle audit may not be installed, or property \"analyzer.bundle.audit.path\" may not be set.", e); } } + /** + * Test Ruby addCriticalityToVulnerability + */ + @Test + public void testAddCriticalityToVulnerability() throws AnalysisException, DatabaseException { + try { + analyzer.initialize(); + + final Dependency result = new Dependency(BaseTest.getResourceAsFile(this, + "ruby/vulnerable/gems/sinatra/Gemfile.lock")); + final Engine engine = new Engine(); + analyzer.analyze(result, engine); + + + Dependency dependency = engine.getDependencies().get(0); + Vulnerability vulnerability = dependency.getVulnerabilities().first(); + assertEquals(vulnerability.getCvssScore(), 5.0f, 0.0); + + } catch (Exception e) { + LOGGER.warn("Exception setting up RubyBundleAuditAnalyzer. Make sure Ruby gem bundle-audit is installed. You may also need to set property \"analyzer.bundle.audit.path\".", e); + Assume.assumeNoException("Exception setting up RubyBundleAuditAnalyzer; bundle audit may not be installed, or property \"analyzer.bundle.audit.path\" may not be set.", e); + } + } + + /** * Test when Ruby bundle-audit is not available on the system. * @@ -137,4 +165,6 @@ public class RubyBundleAuditAnalyzerTest extends BaseTest { LOGGER.info("phantom-bundle-audit is not available. Ruby Bundle Audit Analyzer is disabled as expected."); } } + + } diff --git a/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/nvdcve/CveDBIntegrationTest.java b/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/nvdcve/CveDBIntegrationTest.java index 319136850..01ad0f740 100644 --- a/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/nvdcve/CveDBIntegrationTest.java +++ b/dependency-check-core/src/test/java/org/owasp/dependencycheck/data/nvdcve/CveDBIntegrationTest.java @@ -24,6 +24,8 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; import org.junit.Assert; + +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import org.junit.Test; import org.owasp.dependencycheck.dependency.Vulnerability; @@ -72,6 +74,24 @@ public class CveDBIntegrationTest extends BaseDBTestCase { } } } + /** + * Test of getVulnerability method, of class CveDB. + */ + @Test + public void testgetVulnerability() throws Exception { + CveDB instance = null; + try { + instance = new CveDB(); + instance.open(); + Vulnerability result = instance.getVulnerability("CVE-2015-3225"); + assertTrue(result.getDescription().contains("lib/rack/utils.rb in Rack before 1.5.4 and 1.6.x before 1.6.2")); + + } finally { + if (instance != null) { + instance.close(); + } + } + } /** * Test of getVulnerabilities method, of class CveDB. From 0f37c2b59c0de1dcb4d78139ab51011f3e5c8595 Mon Sep 17 00:00:00 2001 From: Dave Goddard Date: Fri, 29 Apr 2016 16:17:51 -0400 Subject: [PATCH 2/3] Adding sinatra fixture Signed-off-by: Gabriel Ramirez --- .../ruby/vulnerable/gems/sinatra/Gemfile | 4 ++++ .../ruby/vulnerable/gems/sinatra/Gemfile.lock | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 dependency-check-core/src/test/resources/ruby/vulnerable/gems/sinatra/Gemfile create mode 100644 dependency-check-core/src/test/resources/ruby/vulnerable/gems/sinatra/Gemfile.lock diff --git a/dependency-check-core/src/test/resources/ruby/vulnerable/gems/sinatra/Gemfile b/dependency-check-core/src/test/resources/ruby/vulnerable/gems/sinatra/Gemfile new file mode 100644 index 000000000..1f7318b1c --- /dev/null +++ b/dependency-check-core/src/test/resources/ruby/vulnerable/gems/sinatra/Gemfile @@ -0,0 +1,4 @@ +# encoding: utf-8 +source 'https://rubygems.org' + +gem 'sinatra' diff --git a/dependency-check-core/src/test/resources/ruby/vulnerable/gems/sinatra/Gemfile.lock b/dependency-check-core/src/test/resources/ruby/vulnerable/gems/sinatra/Gemfile.lock new file mode 100644 index 000000000..deb44d853 --- /dev/null +++ b/dependency-check-core/src/test/resources/ruby/vulnerable/gems/sinatra/Gemfile.lock @@ -0,0 +1,17 @@ +GEM + remote: https://rubygems.org/ + specs: + rack (1.5.2) + rack-protection (1.5.2) + rack + sinatra (1.4.4) + rack (~> 1.4) + rack-protection (~> 1.4) + tilt (~> 1.3, >= 1.3.4) + tilt (1.4.1) + +PLATFORMS + ruby + +DEPENDENCIES + sinatra From 35ffd56ea9f4dd079e63f2ecf50904d142c67fc5 Mon Sep 17 00:00:00 2001 From: Jeremy Long Date: Sat, 30 Apr 2016 11:20:26 -0400 Subject: [PATCH 3/3] fixed compile issues in PR --- .../analyzer/RubyBundleAuditAnalyzer.java | 71 ++++++++++++------- 1 file changed, 47 insertions(+), 24 deletions(-) diff --git a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java index 55ad6a405..66e4d0157 100644 --- a/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java +++ b/dependency-check-core/src/main/java/org/owasp/dependencycheck/analyzer/RubyBundleAuditAnalyzer.java @@ -32,9 +32,12 @@ import org.slf4j.LoggerFactory; import java.io.*; import java.util.*; +import java.util.logging.Level; +import org.owasp.dependencycheck.data.nvdcve.DatabaseException; /** - * Used to analyze Ruby Bundler Gemspec.lock files utilizing the 3rd party bundle-audit tool. + * Used to analyze Ruby Bundler Gemspec.lock files utilizing the 3rd party + * bundle-audit tool. * * @author Dale Visser */ @@ -59,7 +62,7 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { public static final String ADVISORY = "Advisory: "; public static final String CRITICALITY = "Criticality: "; - public static CveDB CVEDB = new CveDB(); + public CveDB cvedb; //instance.open(); //Vulnerability result = instance.getVulnerability("CVE-2015-3225"); @@ -88,7 +91,7 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { final ProcessBuilder builder = new ProcessBuilder(args); builder.directory(folder); try { - LOGGER.info("Launching: " + args + " from " + folder); + LOGGER.info("Launching: " + args + " from " + folder); return builder.start(); } catch (IOException ioe) { throw new AnalysisException("bundle-audit failure", ioe); @@ -96,23 +99,34 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { } /** - * Initialize the analyzer. In this case, extract GrokAssembly.exe to a temporary location. + * Initialize the analyzer. In this case, extract GrokAssembly.exe to a + * temporary location. * * @throws Exception if anything goes wrong */ @Override public void initializeFileTypeAnalyzer() throws Exception { - // Now, need to see if bundle-audit actually runs from this location. - Process process = null; - try { - process = launchBundleAudit(Settings.getTempDirectory()); - } - catch(AnalysisException ae) { - LOGGER.warn("Exception from bundle-audit process: {}. Disabling {}", ae.getCause(), ANALYZER_NAME); + try { + cvedb = new CveDB(); + cvedb.open(); + } catch (DatabaseException ex) { + LOGGER.warn("Exception opening the database"); + LOGGER.debug("error", ex); setEnabled(false); + throw ex; + } + // Now, need to see if bundle-audit actually runs from this location. + Process process = null; + try { + process = launchBundleAudit(Settings.getTempDirectory()); + } catch (AnalysisException ae) { + LOGGER.warn("Exception from bundle-audit process: {}. Disabling {}", ae.getCause(), ANALYZER_NAME); + setEnabled(false); + cvedb.close(); + cvedb = null; throw ae; - } - + } + int exitValue = process.waitFor(); if (0 == exitValue) { LOGGER.warn("Unexpected exit code from bundle-audit process. Disabling {}: {}", ANALYZER_NAME, exitValue); @@ -140,7 +154,7 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { } } } - + if (isEnabled()) { LOGGER.info(ANALYZER_NAME + " is enabled. It is necessary to manually run \"bundle-audit update\" " + "occasionally to keep its database up to date."); @@ -168,7 +182,8 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { } /** - * Returns the key used in the properties file to reference the analyzer's enabled property. + * Returns the key used in the properties file to reference the analyzer's + * enabled property. * * @return the analyzer's enabled property setting key */ @@ -178,8 +193,9 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { } /** - * If {@link #analyzeFileType(Dependency, Engine)} is called, then we have successfully initialized, and it will be necessary - * to disable {@link RubyGemspecAnalyzer}. + * If {@link #analyzeFileType(Dependency, Engine)} is called, then we have + * successfully initialized, and it will be necessary to disable + * {@link RubyGemspecAnalyzer}. */ private boolean needToDisableGemspecAnalyzer = true; @@ -210,11 +226,11 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { } BufferedReader rdr = null; try { - BufferedReader errReader = new BufferedReader(new InputStreamReader(process.getErrorStream(), "UTF-8")); - while(errReader.ready()) { - String error = errReader.readLine(); - LOGGER.warn(error); - } + BufferedReader errReader = new BufferedReader(new InputStreamReader(process.getErrorStream(), "UTF-8")); + while (errReader.ready()) { + String error = errReader.readLine(); + LOGGER.warn(error); + } rdr = new BufferedReader(new InputStreamReader(process.getInputStream(), "UTF-8")); processBundlerAuditOutput(dependency, engine, rdr); } catch (IOException ioe) { @@ -305,8 +321,15 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer { } else if ("Low".equals(criticality)) { vulnerability.setCvssScore(2.0f); } else { - //vulnerability.getName() - vulnerability.setCvssScore(-1.0f); + try { + //TODO wouldn't we want to do this for all items from bundle-audit? This + //should give a more correct CVSS + Vulnerability v = cvedb.getVulnerability(vulnerability.getName()); + vulnerability.setCvssScore(v.getCvssScore()); + } catch (DatabaseException ex) { + vulnerability.setCvssScore(-1.0f); + LOGGER.debug("Unable to look up vulnerability {}",vulnerability.getName()); + } } } LOGGER.debug(String.format("bundle-audit (%s): %s", parentName, nextLine));