Safety parser: Fix unit tests and add component (#3963)

* Safety parser: Fix unit tests and add component

* Fix deduplication

Author: Damien Carol

dojo/settings/settings.dist.py

@@ -863,6 +863,7 @@ def generate_url(scheme, double_slashes, user, password, host, port, path, param
     'Trivy Scan': DEDUPE_ALGO_HASH_CODE,
     'HackerOne Cases': DEDUPE_ALGO_UNIQUE_ID_FROM_TOOL_OR_HASH_CODE,
     'Snyk Scan': DEDUPE_ALGO_HASH_CODE,
+    'Safety Scan': DEDUPE_ALGO_UNIQUE_ID_FROM_TOOL,
 }
 
 DUPE_DELETE_MAX_PER_RUN = env('DD_DUPE_DELETE_MAX_PER_RUN')

dojo/tools/safety/parser.py

@@ -18,73 +18,63 @@ def get_label_for_scan_types(self, scan_type):
     def get_description_for_scan_types(self, scan_type):
         return "Safety scan (--json) output file can be imported in JSON format."
 
-    def get_findings(self, json_output, test):
-
-        # Grab Safety DB for CVE lookup
+    def get_safetydb(self):
+        """Grab Safety DB for CVE lookup"""
         url = "https://raw.githubusercontent.com/pyupio/safety-db/master/data/insecure_full.json"
         try:
             response = urllib.request.urlopen(url)
-            safety_db = json.loads(response.read().decode('utf-8'))
+            return json.load(response)
         except urllib.error.URLError as e:
             logger.warn("Error Message: %s", e)
             logger.warn("Could not resolve %s. Fallback is using the offline version from dojo/tools/safety/insecure_full.json.", url)
-            with open("dojo/tools/safety/insecure_full.json", "r") as f:
-                safety_db = json.load(f)
-            f.close()
+            with open("dojo/tools/safety/insecure_full.json", "r") as insecure_full:
+                return json.load(insecure_full)
 
-        tree = self.parse_json(json_output)
-        return self.get_items(tree, test, safety_db)
+    def get_findings(self, json_output, test):
+        safety_db = self.get_safetydb()
 
-    def parse_json(self, json_output):
-        data = json_output.read() or '[]'
-        try:
-            json_obj = json.loads(str(data, 'utf-8'))
-        except:
-            json_obj = json.loads(data)
-        tree = {l[4]: {'package': str(l[0]),
-                       'affected': str(l[1]),
-                       'installed': str(l[2]),
-                       'description': str(l[3]),
-                       'id': str(l[4])}
-                for l in json_obj}  # noqa: E741
-        return tree
+        tree = json.load(json_output)
 
-    def get_items(self, tree, test, safety_db):
         items = {}
-
-        for key, node in tree.items():
-            item = get_item(node, test, safety_db)
-            items[key] = item
+        for node in tree:
+            item_node = {
+                'package': str(node[0]),
+                'affected': str(node[1]),
+                'installed': str(node[2]),
+                'description': str(node[3]),
+                'id': str(node[4])
+            }
+            severity = 'Medium'  # Because Safety doesn't include severity rating
+            cve = None
+            for a in safety_db[item_node['package']]:
+                if a['id'] == 'pyup.io-' + item_node['id']:
+                    if a['cve']:
+                        cve = a['cve']
+            title = item_node['package'] + " (" + item_node['affected'] + ")"
+
+            finding = Finding(title=title + " | " + cve if cve else title,
+                            test=test,
+                            severity=severity,
+                            description="**Description:** " + item_node['description'] +
+                                        "\n**Vulnerable Package:** " + item_node['package'] +
+                                        "\n**Installed Version:** " + item_node['installed'] +
+                                        "\n**Vulnerable Versions:** " + item_node['affected'] +
+                                        "\n**CVE:** " + (cve or "N/A") +
+                                        "\n**ID:** " + item_node['id'],
+                            cve=cve,
+                            cwe=1035,  # Vulnerable Third Party Component
+                            mitigation="No mitigation provided",
+                            references="No reference provided",
+                            active=False,
+                            verified=False,
+                            false_p=False,
+                            duplicate=False,
+                            out_of_scope=False,
+                            mitigated=None,
+                            impact="No impact provided",
+                            component_name=item_node['package'],
+                            component_version=item_node['installed'],
+                            unique_id_from_tool=item_node['id'])
+            items[finding.unique_id_from_tool] = finding
 
         return list(items.values())
-
-
-def get_item(item_node, test, safety_db):
-    severity = 'Info'  # Because Safety doesn't include severity rating
-    cve = ''.join(a['cve'] or ''
-                  for a in safety_db[item_node['package']]
-                  if a['id'] == 'pyup.io-' + item_node['id']) or None
-    title = item_node['package'] + " (" + item_node['affected'] + ")"
-
-    finding = Finding(title=title + " | " + cve if cve else title,
-                      test=test,
-                      severity=severity,
-                      description=item_node['description'] +
-                                  "\n Vulnerable Package: " + item_node['package'] +
-                                  "\n Installed Version: " + item_node['installed'] +
-                                  "\n Vulnerable Versions: " + item_node['affected'] +
-                                  "\n CVE: " + (cve or "N/A") +
-                                  "\n ID: " + item_node['id'],
-                      cve=cve,
-                      cwe=1035,  # Vulnerable Third Party Component
-                      mitigation="No mitigation provided",
-                      references="No reference provided",
-                      active=False,
-                      verified=False,
-                      false_p=False,
-                      duplicate=False,
-                      out_of_scope=False,
-                      mitigated=None,
-                      impact="No impact provided")
-
-    return finding

dojo/unittests/scans/safety/empty.json

@@ -0,0 +1 @@
+[]

dojo/unittests/scans/safety/many_vulns.json

@@ -0,0 +1,47 @@
+[
+    [
+        "pyyaml",
+        "<5.4",
+        "5.3.1",
+        "A vulnerability was discovered in the PyYAML library in versions before 5.4, where it is susceptible to arbitrary code execution when it processes untrusted YAML files through the full_load method or with the FullLoader loader. Applications that use the library to process untrusted input may be vulnerable to this flaw. This flaw allows an attacker to execute arbitrary code on the system by abusing the python/object/new constructor. This flaw is due to an incomplete fix for CVE-2020-1747. See CVE-2020-14343.",
+        "39611",
+        null,
+        null
+    ],
+    [
+        "jinja2",
+        ">=0.0.0,<2.11.3",
+        "2.11.1",
+        "This affects the package jinja2 from 0.0.0 and before 2.11.3. The ReDOS vulnerability of the regex is mainly due to the sub-pattern [a-zA-Z0-9._-]+.[a-zA-Z0-9._-]+ This issue can be mitigated by Markdown to format user content instead of the urlize filter, or by implementing request timeouts and limiting process memory. See CVE-2020-28493.",
+        "39525",
+        null,
+        null
+    ],
+    [
+        "httplib2",
+        "<0.19.0",
+        "0.18.1",
+        "httplib2 is a comprehensive HTTP client library for Python. In httplib2 before version 0.19.0, a malicious server which responds with long series of \"\\xa0\" characters in the \"www-authenticate\" header may cause Denial of Service (CPU burn while parsing header) of the httplib2 client accessing said server. This is fixed in version 0.19.0 which contains a new implementation of auth headers parsing using the pyparsing library. See CVE-2021-21240.",
+        "39608",
+        null,
+        null
+    ],
+    [
+        "django",
+        "==2.2.17",
+        "2.2.17",
+        "Django 2.2.18 fixes a security issue with severity \"low\" in 2.2.17 (CVE-2021-3281).",
+        "39523",
+        null,
+        null
+    ],
+    [
+        "django",
+        ">=2.2,<2.2.18",
+        "2.2.17",
+        "In Django 2.2 before 2.2.18, 3.0 before 3.0.12, and 3.1 before 3.1.6, the django.utils.archive.extract method (used by \"startapp --template\" and \"startproject --template\") allows directory traversal via an archive with absolute paths or relative paths with dot segments. See CVE-2021-3281.",
+        "39526",
+        null,
+        null
+    ]
+]

dojo/unittests/tools/test_safety_parser.py

@@ -31,4 +31,19 @@ def test_multiple_cves(self):
         parser = SafetyParser()
         findings = parser.get_findings(testfile, Test())
         self.assertEqual(1, len(findings))
-        self.assertEqual("CVE-2019-12385", findings[0].cve)
+        for finding in findings:
+            if "37863" == finding.unique_id_from_tool:
+                self.assertIsNone(finding.cve)
+
+    def test_multiple2(self):
+        testfile = open("dojo/unittests/scans/safety/many_vulns.json")
+        parser = SafetyParser()
+        findings = parser.get_findings(testfile, Test())
+        self.assertEqual(5, len(findings))
+        for finding in findings:
+            if "39608" == finding.unique_id_from_tool:
+                self.assertEqual("httplib2", finding.component_name)
+                self.assertEqual("0.18.1", finding.component_version)
+                self.assertEqual("CVE-2021-21240", finding.cve)
+            elif "39525" == finding.unique_id_from_tool:
+                self.assertIsNone(finding.cve)