Merge branch 'geramirez-fix-cvss-for-bundle-audit'

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;
@@ -31,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
  */
@@ -58,6 +62,10 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer {
     public static final String ADVISORY = "Advisory: ";
     public static final String CRITICALITY = "Criticality: ";
 
+    public CveDB cvedb;
+    //instance.open();
+    //Vulnerability result = instance.getVulnerability("CVE-2015-3225");
+
     /**
      * @return a filter that accepts files named Gemfile.lock
      */
@@ -83,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);
@@ -91,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.
+        try {
-    	Process process = null;
+            cvedb = new CveDB();
-    	try {
+            cvedb.open();
-	        process = launchBundleAudit(Settings.getTempDirectory());
+        } catch (DatabaseException ex) {
-    	}
+            LOGGER.warn("Exception opening the database");
-    	catch(AnalysisException ae) {
+            LOGGER.debug("error", ex);
-    		LOGGER.warn("Exception from bundle-audit process: {}. Disabling {}", ae.getCause(), ANALYZER_NAME);
             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);
@@ -135,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.");
@@ -163,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
      */
@@ -173,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
+     * If {@link #analyzeFileType(Dependency, Engine)} is called, then we have
-     * to disable {@link RubyGemspecAnalyzer}.
+     * successfully initialized, and it will be necessary to disable
+     * {@link RubyGemspecAnalyzer}.
      */
     private boolean needToDisableGemspecAnalyzer = true;
 
@@ -205,11 +226,11 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer {
         }
         BufferedReader rdr = null;
         try {
-        	BufferedReader errReader = new BufferedReader(new InputStreamReader(process.getErrorStream(), "UTF-8"));
+            BufferedReader errReader = new BufferedReader(new InputStreamReader(process.getErrorStream(), "UTF-8"));
-        	while(errReader.ready()) {
+            while (errReader.ready()) {
-        		String error = errReader.readLine();
+                String error = errReader.readLine();
-        		LOGGER.warn(error);
+                LOGGER.warn(error);
-        	}
+            }
             rdr = new BufferedReader(new InputStreamReader(process.getInputStream(), "UTF-8"));
             processBundlerAuditOutput(dependency, engine, rdr);
         } catch (IOException ioe) {
@@ -300,7 +321,15 @@ public class RubyBundleAuditAnalyzer extends AbstractFileTypeAnalyzer {
             } else if ("Low".equals(criticality)) {
                 vulnerability.setCvssScore(2.0f);
             } else {
-                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));

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;

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.");
         }
     }
+
+
 }

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.

dependency-check-core/src/test/resources/ruby/vulnerable/gems/sinatra/Gemfile

@@ -0,0 +1,4 @@
+# encoding: utf-8
+source 'https://rubygems.org'
+
+gem 'sinatra'

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