Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix sonar issues and improved test coverage #36

Merged
merged 1 commit into from
Apr 25, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
target/
*.iml
.idea
.idea
.DS_Store
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@
import org.xml.sax.SAXException;

import javax.xml.parsers.ParserConfigurationException;
import java.io.*;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.InputStream;
import java.io.Serializable;

public class DependencyCheckSensor implements Sensor {

Expand Down Expand Up @@ -106,6 +109,8 @@ private void incrementCount(Severity severity) {
case MINOR:
this.minorIssuesCount++;
break;
default:
LOGGER.debug("Unknown severity {}", severity);
}
}

Expand All @@ -114,7 +119,7 @@ private void addIssues(SensorContext context, Analysis analysis) {
return;
}
for (Dependency dependency : analysis.getDependencies()) {
InputFile testFile = fileSystem.inputFile(fileSystem.predicates().is(new File(dependency.getFilePath())));
InputFile testFile = fileSystem.inputFile(fileSystem.predicates().hasPath(dependency.getFilePath()));

int depVulnCount = dependency.getVulnerabilities().size();

Expand Down Expand Up @@ -174,7 +179,7 @@ public void execute(SensorContext sensorContext) {
profiler.startInfo("Process Dependency-Check report");
try {
Analysis analysis = parseAnalysis(sensorContext);
totalDependencies = analysis.getDependencies().size();
this.totalDependencies = analysis.getDependencies().size();
addIssues(sensorContext, analysis);
} catch (FileNotFoundException e) {
LOGGER.debug("Analysis aborted due to missing report file", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

public final class DependencyCheckMetrics implements Metrics {

public static final String DOMAIN = "OWASP-Dependency-Check";
private static final String DOMAIN = "OWASP-Dependency-Check";

private static final String INHERITED_RISK_SCORE_KEY = "inherited_risk_score";
private static final String VULNERABLE_COMPONENT_RATIO_KEY = "vulnerable_component_ratio";
Expand All @@ -40,19 +40,6 @@ public final class DependencyCheckMetrics implements Metrics {
private static final String MEDIUM_SEVERITY_VULNS_KEY = "medium_severity_vulns";
private static final String LOW_SEVERITY_VULNS_KEY = "low_severity_vulns";


public static int inheritedRiskScore(int high, int medium, int low) {
return (high * 5) + (medium * 3) + (low);
}

public static double vulnerableComponentRatio(int vulnerabilities, int vulnerableComponents) {
double ratio = 0.0;
if(vulnerableComponents > 0) {
ratio = (double) vulnerabilities / vulnerableComponents;
}
return ratio;
}

public static final Metric<Serializable> INHERITED_RISK_SCORE = new Metric.Builder(DependencyCheckMetrics.INHERITED_RISK_SCORE_KEY, "Inherited Risk Score", Metric.ValueType.INT)
.setDescription("Inherited Risk Score")
.setDirection(Metric.DIRECTION_BETTER)
Expand Down Expand Up @@ -122,6 +109,19 @@ public static double vulnerableComponentRatio(int vulnerabilities, int vulnerabl
.setHidden(false)
.create();

public static double vulnerableComponentRatio(int vulnerabilities, int vulnerableComponents) {
double ratio = 0.0;
if(vulnerableComponents > 0) {
ratio = (double) vulnerabilities / vulnerableComponents;
}
return ratio;
}

public static int inheritedRiskScore(int high, int medium, int low) {
return (high * 5) + (medium * 3) + (low);
}


@Override
public List<Metric> getMetrics() {
return Arrays.asList(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
public class KnownCveRuleDefinition implements RulesDefinition {


@Override
@ParametersAreNonnullByDefault
public void define(Context context) {
NewRepository repo = context.createRepository(DependencyCheckPlugin.REPOSITORY_KEY, DependencyCheckPlugin.LANGUAGE_KEY);
Expand All @@ -52,8 +53,6 @@ public void define(Context context) {
rule.setHtmlDescription(description);

// There's simply no way to know how much effort will be involved in updating/replacing a vulnerable component
//rule.setDebtSubCharacteristic(RulesDefinition.SubCharacteristics.SECURITY_COMPLIANCE);
//rule.setDebtRemediationFunction(rule.debtRemediationFunctions().linearWithOffset("1h", "30min"));

repo.done();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ public NeutralLanguage() {
super(DependencyCheckPlugin.LANGUAGE_KEY, "Neutral");
}

@Override
public String[] getFileSuffixes() {
return new String[0];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,33 +22,102 @@
import org.junit.Before;
import org.junit.Test;
import org.sonar.api.batch.fs.FileSystem;
import org.sonar.api.batch.fs.InputComponent;
import org.sonar.api.batch.measure.Metric;
import org.sonar.api.batch.sensor.SensorContext;
import org.sonar.api.batch.sensor.SensorDescriptor;
import org.sonar.api.scan.filesystem.PathResolver;
import org.sonar.dependencycheck.DependencyCheckSensor;
import org.sonar.dependencycheck.base.DependencyCheckConstants;

import java.io.File;
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.file.Paths;

import static org.fest.assertions.Assertions.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.*;

public class DependencyCheckSensorTest {
private FileSystem fileSystem;
private PathResolver pathResolver;
private DependencyCheckSensor sensor;

private File sampleReport;

@Before
public void init() {
this.fileSystem = mock(FileSystem.class);
public void init() throws URISyntaxException {
this.fileSystem = mock(FileSystem.class, RETURNS_DEEP_STUBS);
this.pathResolver = mock(PathResolver.class);
this.sensor = new DependencyCheckSensor(this.fileSystem, this.pathResolver);

// mock a sample report
final URL sampleResourceURI = getClass().getClassLoader().getResource("report/dependency-check-report.xml");
assert sampleResourceURI != null;
this.sampleReport = Paths.get(sampleResourceURI.toURI()).toFile();
}

@Test
public void toStringTest() {
assertThat(this.sensor.toString()).isEqualTo("OWASP Dependency-Check");
}

@Test
public void testDescribe() {
final SensorDescriptor descriptor = mock(SensorDescriptor.class);
sensor.describe(descriptor);
verify(descriptor).name("OWASP Dependency-Check");
}
@Test
public void shouldAnalyse() throws URISyntaxException {
//todo: Once the Sensor is capable of working properly, populate this unit test.
final SensorContext context = mock(SensorContext.class, RETURNS_DEEP_STUBS);

when(context.settings().getString(DependencyCheckConstants.REPORT_PATH_PROPERTY)).thenReturn("dependency-check-report.xml");
when(pathResolver.relativeFile(any(File.class), anyString())).thenReturn(sampleReport);
sensor.execute(context);
}

@Test
public void shouldSkipIfReportWasNotFound() throws URISyntaxException {
final SensorContext context = mock(SensorContext.class, RETURNS_DEEP_STUBS);

when(pathResolver.relativeFile(any(File.class), anyString())).thenReturn(null);
sensor.execute(context);
verify(context, never()).newIssue();
}

@Test
public void shouldAddAnIssueForAVulnerability() throws URISyntaxException {
final SensorContext context = mock(SensorContext.class, RETURNS_DEEP_STUBS);

when(context.settings().getString(DependencyCheckConstants.REPORT_PATH_PROPERTY)).thenReturn("dependency-check-report.xml");
when(pathResolver.relativeFile(any(File.class), anyString())).thenReturn(sampleReport);
sensor.execute(context);

verify(context, times(3)).newIssue();
}

@Test
public void shouldPersistTotalMetrics() throws URISyntaxException {
final SensorContext context = mock(SensorContext.class, RETURNS_DEEP_STUBS);

when(context.settings().getString(DependencyCheckConstants.REPORT_PATH_PROPERTY)).thenReturn("dependency-check-report.xml");
when(pathResolver.relativeFile(any(File.class), anyString())).thenReturn(sampleReport);
sensor.execute(context);

verify(context.newMeasure(), times(8)).forMetric(any(Metric.class));
}

@Test
public void shouldPersistMetricsOnReport() throws URISyntaxException {
final SensorContext context = mock(SensorContext.class, RETURNS_DEEP_STUBS);

when(context.settings().getString(DependencyCheckConstants.REPORT_PATH_PROPERTY)).thenReturn("dependency-check-report.xml");
when(pathResolver.relativeFile(any(File.class), anyString())).thenReturn(sampleReport);
sensor.execute(context);

verify(context.newMeasure(), atLeastOnce()).on(any(InputComponent.class));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public void parseReport() throws Exception {
assertThat(analysis.getProjectInfo().getCredits()).isEqualTo("This report contains data retrieved from the National Vulnerability Database: http://nvd.nist.gov");

Collection<Dependency> dependencies = analysis.getDependencies();
assertThat(dependencies.size()).isEqualTo(5);
assertThat(dependencies).hasSize(5);
Iterator iterator = dependencies.iterator();
Dependency dependency = (Dependency) iterator.next();

Expand All @@ -68,31 +68,31 @@ public void parseReport() throws Exception {
assertThat(vulnerability.getName()).isEqualTo("CVE-2014-3596");
assertThat(vulnerability.getCvssScore()).isEqualTo("5.8");
assertThat(vulnerability.getSeverity()).isEqualTo("Medium");
assertThat(vulnerability.getCwe() == null);
assertThat(vulnerability.getCwe()).isNull();
assertThat(vulnerability.getDescription()).isEqualTo("The getCN function in Apache Axis 1.4 and earlier does not properly verify that the server hostname matches a domain name in the subject's Common Name (CN) or subjectAltName field of the X.509 certificate, which allows man-in-the-middle attackers to spoof SSL servers via a certificate with a subject that specifies a common name in a field that is not the CN field. NOTE: this issue exists because of an incomplete fix for CVE-2012-5784.");

vulnerability = (Vulnerability) vulnIterator.next();
assertThat(vulnerability.getName()).isEqualTo("CVE-2012-5784");
assertThat(vulnerability.getCvssScore()).isEqualTo("5.8");
assertThat(vulnerability.getSeverity()).isEqualTo("Medium");
assertThat(vulnerability.getCwe().equals("CWE-20 Improper Input Validation"));
assertThat(vulnerability.getCwe()).isEqualTo("CWE-20 Improper Input Validation");
assertThat(vulnerability.getDescription()).isEqualTo("Apache Axis 1.4 and earlier, as used in PayPal Payments Pro, PayPal Mass Pay, PayPal Transactional Information SOAP, the Java Message Service implementation in Apache ActiveMQ, and other products, does not verify that the server hostname matches a domain name in the subject's Common Name (CN) or subjectAltName field of the X.509 certificate, which allows man-in-the-middle attackers to spoof SSL servers via an arbitrary valid certificate.");

dependency = (Dependency) iterator.next();
assertThat(dependency.getEvidenceCollected().size()).isEqualTo(13);
assertThat(dependency.getVulnerabilities().size()).isEqualTo(0);

dependency = (Dependency) iterator.next();
assertThat(dependency.getEvidenceCollected().size()).isEqualTo(12);
assertThat(dependency.getVulnerabilities().size()).isEqualTo(0);
assertThat(dependency.getEvidenceCollected()).hasSize(12);
assertThat(dependency.getVulnerabilities()).isEmpty();

dependency = (Dependency) iterator.next();
assertThat(dependency.getEvidenceCollected().size()).isEqualTo(32);
assertThat(dependency.getVulnerabilities().size()).isEqualTo(1);
assertThat(dependency.getEvidenceCollected()).hasSize(32);
assertThat(dependency.getVulnerabilities()).hasSize(1);

dependency = (Dependency) iterator.next();
assertThat(dependency.getEvidenceCollected().size()).isEqualTo(9);
assertThat(dependency.getVulnerabilities().size()).isEqualTo(0);
assertThat(dependency.getEvidenceCollected()).hasSize(9);
assertThat(dependency.getVulnerabilities()).isEmpty();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Dependency-Check Plugin for SonarQube
* Copyright (C) 2015-2017 Steve Springett
* steve.springett@owasp.org
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.dependencycheck.rule;

import org.junit.Test;
import org.mockito.InOrder;
import org.sonar.api.server.rule.RulesDefinition;
import org.sonar.dependencycheck.DependencyCheckPlugin;

import static org.mockito.Matchers.anyString;
import static org.mockito.Mockito.*;

/**
* @author Gregor Tudan, Cofinpro AG
*/
public class KnownCveRuleDefinitionTest {

private KnownCveRuleDefinition rule = new KnownCveRuleDefinition();

@Test
public void define() throws Exception {
final RulesDefinition.Context context = mock(RulesDefinition.Context.class);
final RulesDefinition.NewRepository repo = mock(RulesDefinition.NewRepository.class);
final RulesDefinition.NewRule rule = mock(RulesDefinition.NewRule.class, RETURNS_SMART_NULLS);

when(repo.createRule(DependencyCheckPlugin.RULE_KEY)).thenReturn(rule);
when(context.createRepository(anyString(), anyString())).thenReturn(repo);

this.rule.define(context);

InOrder inOrder = inOrder(context, repo);

inOrder.verify(context).createRepository("OWASP","neutral");
inOrder.verify(repo).createRule(DependencyCheckPlugin.RULE_KEY);

inOrder.verify(repo).done();
inOrder.verifyNoMoreInteractions();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Dependency-Check Plugin for SonarQube
* Copyright (C) 2015-2017 Steve Springett
* steve.springett@owasp.org
*
* This program is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3 of the License, or (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public License
* along with this program; if not, write to the Free Software Foundation,
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/
package org.sonar.dependencycheck.rule;

import org.junit.Test;

import static org.fest.assertions.Assertions.assertThat;


/**
* @author Gregor Tudan, Cofinpro AG
*/
public class NeutralLanguageTest {
@Test
public void getFileSuffixes() throws Exception {
assertThat(new NeutralLanguage().getFileSuffixes()).isEmpty();
}

}
Loading