Merge pull request #2492 from snyk/fix/critical-level-sarif

fix: critical level is considered error in sarif
snyk · Jan 5, 2022 · eec58db · eec58db
2 parents 0875bb3 + a034e57
commit eec58db
Show file tree

Hide file tree

Showing 5 changed files with 122 additions and 17 deletions.
diff --git a/src/lib/formatters/iac-output.ts b/src/lib/formatters/iac-output.ts
@@ -12,11 +12,11 @@ import {
 import { printPath } from './remediation-based-format-issues';
 import { titleCaseText } from './legacy-format-issue';
 import * as sarif from 'sarif';
-import { SEVERITY } from '../../lib/snyk-test/legacy';
 import { colorTextBySeverity } from '../../lib/snyk-test/common';
 import { IacFileInDirectory } from '../../lib/types';
 import { isLocalFolder } from '../../lib/detect';
 import { getSeverityValue } from './get-severity-value';
+import { getIssueLevel } from './sarif-output';
 const debug = Debug('iac-output');
 
 function formatIacIssue(
@@ -177,12 +177,6 @@ export function createSarifOutputForIac(
   };
 }
 
-function getIssueLevel(
-  severity: SEVERITY | 'none',
-): sarif.ReportingConfiguration.level {
-  return severity === SEVERITY.HIGH ? 'error' : 'warning';
-}
-
 export function extractReportingDescriptor(
   results: ResponseIssues,
 ): sarif.ReportingDescriptor[] {

diff --git a/src/lib/formatters/sarif-output.ts b/src/lib/formatters/sarif-output.ts
@@ -1,5 +1,6 @@
 import * as sarif from 'sarif';
 import { TestResult } from '../snyk-test/legacy';
+import { SEVERITY } from '../snyk-test/legacy';
 const upperFirst = require('lodash.upperfirst');
 
 export function createSarifOutputForContainers(
@@ -20,6 +21,14 @@ export function createSarifOutputForContainers(
   return sarifRes;
 }
 
+export function getIssueLevel(
+  severity: SEVERITY | 'none',
+): sarif.ReportingConfiguration.level {
+  return severity === SEVERITY.HIGH || severity === SEVERITY.CRITICAL
+    ? 'error'
+    : 'warning';
+}
+
 export function getTool(testResult): sarif.Tool {
   const tool: sarif.Tool = {
     driver: {
@@ -38,7 +47,7 @@ export function getTool(testResult): sarif.Tool {
       if (pushedIds[vuln.id]) {
         return;
       }
-      const level = vuln.severity === 'high' ? 'error' : 'warning';
+      const level = getIssueLevel(vuln.severity);
       const cve = vuln['identifiers']['CVE'][0];
       pushedIds[vuln.id] = true;
       return {

diff --git a/test/jest/unit/lib/formatters/__snapshots__/sarif-output.spec.ts.snap b/test/jest/unit/lib/formatters/__snapshots__/sarif-output.spec.ts.snap
@@ -1,6 +1,83 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`createSarifOutputForContainers general 1`] = `
+exports[`createSarifOutputForContainers general with critical severity issue 1`] = `
+Object {
+  "runs": Array [
+    Object {
+      "results": Array [
+        Object {
+          "locations": Array [
+            Object {
+              "physicalLocation": Object {
+                "artifactLocation": Object {
+                  "uri": undefined,
+                },
+                "region": Object {
+                  "startLine": 1,
+                },
+              },
+            },
+          ],
+          "message": Object {
+            "text": "This file introduces a vulnerable expat package with a critical severity vulnerability.",
+          },
+          "ruleId": "SNYK-LINUX-EXPAT-450908",
+        },
+      ],
+      "tool": Object {
+        "driver": Object {
+          "name": "Snyk Container",
+          "rules": Array [
+            Object {
+              "defaultConfiguration": Object {
+                "level": "error",
+              },
+              "fullDescription": Object {
+                "text": "(CVE-2018-20843) expat@2.2.5-r0",
+              },
+              "help": Object {
+                "markdown": "## Overview
+In libexpat in Expat before 2.2.7, XML input including XML names that contain a large number of colons could make the XML parser consume a high amount of RAM and CPU resources while processing (enough to be usable for denial-of-service attacks).
+
+## References
+- [Bugtraq Mailing List](https://seclists.org/bugtraq/2019/Jun/39)
+- [CVE Details](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-20843)
+- [Debian Security Advisory](https://www.debian.org/security/2019/dsa-4472)
+- [Debian Security Announcement](https://lists.debian.org/debian-lts-announce/2019/06/msg00028.html)
+- [Debian Security Tracker](https://security-tracker.debian.org/tracker/CVE-2018-20843)
+- [GitHub Commit](https://github.com/libexpat/libexpat/pull/262/commits/11f8838bf99ea0a6f0b76f9760c43704d00c4ff6)
+- [GitHub Issue](https://github.com/libexpat/libexpat/issues/186)
+- [GitHub PR](https://github.com/libexpat/libexpat/pull/262)
+- [MISC](https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=5226)
+- [MISC](https://github.com/libexpat/libexpat/blob/R_2_2_7/expat/Changes)
+- [Netapp Security Advisory](https://security.netapp.com/advisory/ntap-20190703-0001/)
+- [Ubuntu CVE Tracker](http://people.ubuntu.com/~ubuntu-security/cve/CVE-2018-20843)
+- [Ubuntu Security Advisory](https://usn.ubuntu.com/4040-1/)
+- [Ubuntu Security Advisory](https://usn.ubuntu.com/4040-2/)
+",
+                "text": "",
+              },
+              "id": "SNYK-LINUX-EXPAT-450908",
+              "properties": Object {
+                "tags": Array [
+                  "security",
+                  "CWE-611",
+                ],
+              },
+              "shortDescription": Object {
+                "text": "Critical severity - XML External Entity (XXE) Injection vulnerability in expat",
+              },
+            },
+          ],
+        },
+      },
+    },
+  ],
+  "version": "2.1.0",
+}
+`;
+
+exports[`createSarifOutputForContainers general with high severity issue 1`] = `
 Object {
   "runs": Array [
     Object {

diff --git a/test/jest/unit/lib/formatters/iac-output.spec.ts b/test/jest/unit/lib/formatters/iac-output.spec.ts
@@ -7,12 +7,13 @@ import { SEVERITY } from '../../../../../src/lib/snyk-test/legacy';
 
 describe('createSarifOutputForIac', () => {
   function createResponseIssue(
+    severity = SEVERITY.HIGH,
     issueOverrides?: Partial<AnnotatedIacIssue>,
   ): IacTestResponse {
     const issue: AnnotatedIacIssue = {
       id: 'ID',
       title: 'TITLE',
-      severity: SEVERITY.HIGH,
+      severity,
       isIgnored: false,
       cloudConfigPath: ['resource', 'something'],
       subType: 'SUBTYPE',
@@ -49,8 +50,26 @@ describe('createSarifOutputForIac', () => {
     };
   }
 
+  it('treats a high severity issue as an error', () => {
+    const issue = createResponseIssue(SEVERITY.HIGH);
+    const sarif = createSarifOutputForIac([issue]);
+
+    const issueLevel =
+      sarif.runs?.[0]?.tool?.driver?.rules?.[0]?.defaultConfiguration?.level;
+    expect(issueLevel).toEqual('error');
+  });
+
+  it('treats a critical severity issue as an error', () => {
+    const issue = createResponseIssue(SEVERITY.CRITICAL);
+    const sarif = createSarifOutputForIac([issue]);
+
+    const issueLevel =
+      sarif.runs?.[0]?.tool?.driver?.rules?.[0]?.defaultConfiguration?.level;
+    expect(issueLevel).toEqual('error');
+  });
+
   it('includes an artifactLocation and region', () => {
-    const issue = createResponseIssue();
+    const issue = createResponseIssue(SEVERITY.HIGH);
     const sarif = createSarifOutputForIac([issue]);
 
     const location = sarif.runs?.[0]?.results?.[0]?.locations?.[0];
@@ -64,7 +83,7 @@ describe('createSarifOutputForIac', () => {
   });
 
   it('excludes the region if no line number was found', () => {
-    const issue = createResponseIssue({ lineNumber: -1 });
+    const issue = createResponseIssue(SEVERITY.HIGH, { lineNumber: -1 });
     const sarif = createSarifOutputForIac([issue]);
 
     const location = sarif.runs?.[0]?.results?.[0]?.locations?.[0];
@@ -76,7 +95,7 @@ describe('createSarifOutputForIac', () => {
   });
 
   it('excludes the region if no line number is present', () => {
-    const issue = createResponseIssue({ lineNumber: undefined });
+    const issue = createResponseIssue(SEVERITY.HIGH, { lineNumber: undefined });
     const sarif = createSarifOutputForIac([issue]);
 
     const location = sarif.runs?.[0]?.results?.[0]?.locations?.[0];

diff --git a/test/jest/unit/lib/formatters/sarif-output.spec.ts b/test/jest/unit/lib/formatters/sarif-output.spec.ts
@@ -3,14 +3,20 @@ import { SEVERITY, TestResult } from '../../../../../src/lib/snyk-test/legacy';
 import { SupportedProjectTypes } from '../../../../../src/lib/types';
 
 describe('createSarifOutputForContainers', () => {
-  it('general', () => {
-    const testFile = getTestResult();
+  it('general with high severity issue', () => {
+    const testFile = getTestResult(SEVERITY.HIGH);
+    const sarif = createSarifOutputForContainers([testFile]);
+    expect(sarif).toMatchSnapshot();
+  });
+
+  it('general with critical severity issue', () => {
+    const testFile = getTestResult(SEVERITY.CRITICAL);
     const sarif = createSarifOutputForContainers([testFile]);
     expect(sarif).toMatchSnapshot();
   });
 });
 
-function getTestResult(): TestResult {
+function getTestResult(severity: SEVERITY): TestResult {
   return {
     vulnerabilities: [
       {
@@ -63,7 +69,7 @@ function getTestResult(): TestResult {
           },
           vulnerable: ['<2.2.7-r0'],
         },
-        severity: SEVERITY.HIGH,
+        severity,
         title: 'XML External Entity (XXE) Injection',
         from: [
           'docker-image|garethr/snyky@alpine',