Collect vulnerabilities from arch linux by lohani2280 · Pull Request #33 · aboutcode-org/vulnerablecode

lohani2280 · 2019-02-28T01:57:30Z

Refers #20

pombredanne

Thank you!
Could you use the imperative style in your commit message? See https://github.com/nexB/aboutcode/wiki/Writing-good-commit-messages

Something such as Collect vulnerabilities from arch linux #20 (with trailing issue number)

Also what about adding a few unit tests (that do not depend on network being available and run offline)

Thank you again!

lohani2280 · 2019-02-28T23:05:17Z

Thanks for the review. I'll add the suggested changes and update the PR soon.

pombredanne

Thanks!
See my comments inline.

pombredanne · 2019-03-06T12:26:32Z

+    """
+    for data in extract_data:
+        vulnerability = Vulnerability.objects.create(
+            summary=data.get('description', ''),


You use get() and therefore can create things with no values... it does not make sense IMHO. Also using None is fine. No need to put a default of ''

pombredanne · 2019-03-06T12:26:54Z

+        )
+        VulnerabilityReference.objects.create(
+            vulnerability=vulnerability,
+            reference_id=data.get('vulnerability_id', ''),


I am not sure we ever want to create a reference without a reference id/url/source/etc ... same issues as above wrt get()

pombredanne · 2019-03-06T12:28:24Z

+                'affected_version': item["affected"],
+                'fixed_version': item["fixed"]
+            })
+    return package_vulnerabilities


Why not yield instead? This is not stored anywhere is it?

@pombredanne Well I am returning it just to maintain the consistency of the methods with other implmented scrapers like ubuntu and debian.

@lohani2280 ok, there is no need for consistency... do what is right instead. And we can update the other data collectors later. But this is OK as is just now.

pombredanne · 2019-03-06T12:30:32Z

+    package_vulnerabilities = []
+
+    for item in arch_data:
+        if not item["name"] or not item["packages"][0] or not item["fixed"]:


what is item["packages"][0] ? it might be best to use a variable name for this

Why do you skip vulnerabilities are do not have a fixed attribute

pombredanne · 2019-03-06T12:32:01Z

+        if not item["name"] or not item["packages"][0] or not item["fixed"]:
+            continue
+
+        package_vulnerabilities.append({


It would be best to use a proper object rather than a dict for this IMHO. Something like an attr thing (See scancode for usage)

pombredanne · 2019-03-06T12:36:29Z

@@ -0,0 +1,56 @@
+[
+  {
+    "name": "AVG-837",


If you were to explain this record in plain english, what would say? How many vulnerabilities references do we have in there? I think you are missing out all the CVE references and the fact these are arch linux packages

lohani2280 · 2019-03-10T08:20:00Z

@pombredanne I have updated the PR and have tried to address your last reviews. However, I have not updated the unittest. I just want you to review it once and if the changes are as per your expectations then I'll update the PR with unittest as well.

pombredanne

Thank you! see my review comments.

pombredanne · 2019-03-10T17:29:46Z

+                'affected_version': item["affected"],
+                'fixed_version': item["fixed"]
+            })
+    return package_vulnerabilities


@lohani2280 ok, there is no need for consistency... do what is right instead. And we can update the other data collectors later. But this is OK as is just now.

pombredanne · 2019-03-10T17:33:27Z

+
+    for item in arch_data:
+        vulnerabilities = item["issues"]
+        package_name = item["packages"][0]


You should check if there is not exactly one element in the packages array and raise an Exception if so... this way this becomes more robust if this changes in the future and we will be alerted to the change by the failure

Also can you use single quotes for texts unless this is :

a docstring

a string with single quotes

This is for the whole PR.

@pombredanne I have followed your suggestions and raised an Exception if there is not exactly one element in the packages array. I found that there are already some entries having packages array with more than one element in it. For example:

{ "name": "AVG-458", "packages": [ "libmupdf", "mupdf", "mupdf-tools", "mupdf-gl" ], "status": "Fixed", "severity": "High", "type": "arbitrary code execution", "affected": "1.11-4", "fixed": "1.11-5", "ticket": null, "issues": [ "CVE-2017-15587", "CVE-2017-14687", "CVE-2017-14686", "CVE-2017-14685" ], "advisories": [ "ASA-201711-2", "ASA-201711-4", "ASA-201711-1", "ASA-201711-3" ] }

So, should I raise an exception and skip these data entry from dumping into our database?. Please suggest your views on this.

You should not raise an exception then... but instead handle all the data

pombredanne · 2019-03-10T17:35:11Z

+            continue
+
+        for vulnerability in vulnerabilities:
+            if not affected_version:


Why do you skip things here? this is not clear to me. Especially affected_version is NOT part of the vulnerabilities

@pombredanne This was unintended. However, I am skipping the fixed_version as I found many instances in the archlinux security tracker data where fixed_version is null and affected_vesion is not null. For example:

{ "name": "AVG-902", "packages": [ "tcpreplay" ], "status": "Unknown", "severity": "High", "type": "multiple issues", "affected": "4.3.1-1", "fixed": null, "ticket": null, "issues": [ "CVE-2019-8381", "CVE-2019-8377", "CVE-2019-8376" ], "advisories": [] }

We would be collecting CVEs corresponding to affected_version so I thought of skipping the same CVEs corresponding to those null fixed_version.

pombredanne

So I am not sure I get your processing ....

Let's take a simple example with this snippet for a single vulnerability in the JSON feed:

  {
    "name": "AVG-708",
    "packages": [
      "wireshark-common",
      "wireshark-gtk",
      "wireshark-cli",
      "wireshark-qt"
    ],
    "status": "Fixed",
    "severity": "Critical",
    "type": "multiple issues",
    "affected": "2.6.0-1",
    "fixed": "2.6.1-1",
    "ticket": null,
    "issues": [
      "CVE-2018-11362",
      "CVE-2018-11361",
      "CVE-2018-11360",
      "CVE-2018-11359",
      "CVE-2018-11358",
      "CVE-2018-11357",
      "CVE-2018-11356",
      "CVE-2018-11355",
      "CVE-2018-11354"
    ],
    "advisories": [
      "ASA-201805-25",
      "ASA-201805-24",
      "ASA-201805-22",
      "ASA-201805-23"
    ]
  }

Based on this can you tell me what should be the final state of database records that should exist once this has been processed?

This will help me understand your approach.

pombredanne · 2019-03-13T09:28:23Z

+def extract_vulnerabilities(arch_data):
+    """
+    Return a sequence of mappings for each existing combination of
+    package and vulnerability from a mapping of arch linux vulnerabilities


arch_data is not a mapping? seems to be a sequence instead, right?

Sorry, I forgot to update it. Will update in next sync.

pombredanne · 2019-03-13T09:32:30Z

+
+
+ARCHLINUX_TRACKER_URL = 'https://security.archlinux.org/json'
+


What about the advisories at https://security.archlinux.org/advisory/json ?

@pombredanne I didn't see this link earlier. So, I was not including the advisories in my final scraped data. However, I plan to include it now.

OK, that can be in a second step too. I posted #20 (comment) as a reminder

pombredanne · 2019-03-13T09:39:06Z

+
+            for vulnerability in vulnerabilities:
+                if not fixed_version:
+                    fixed_version = ''


Why would you care about an empty string vs. a None?

Actually fixed_version attribute contains null value which on dumping in database throws error so I thought to convert the null into an empty string before dumping into the database.

pombredanne · 2019-03-13T09:42:47Z

@@ -0,0 +1,56 @@
+[
+  {
+    "name": "AVG-837",


lohani2280 · 2019-03-13T12:52:26Z

@pombredanne

So I am not sure I get your processing ....

Let's take a simple example with this snippet for a single vulnerability in the JSON feed:
  {
    "name": "AVG-708",
    "packages": [
      "wireshark-common",
      "wireshark-gtk",
      "wireshark-cli",
      "wireshark-qt"
    ],
    "status": "Fixed",
    "severity": "Critical",
    "type": "multiple issues",
    "affected": "2.6.0-1",
    "fixed": "2.6.1-1",
    "ticket": null,
    "issues": [
      "CVE-2018-11362",
      "CVE-2018-11361",
      "CVE-2018-11360",
      "CVE-2018-11359",
      "CVE-2018-11358",
      "CVE-2018-11357",
      "CVE-2018-11356",
      "CVE-2018-11355",
      "CVE-2018-11354"
    ],
    "advisories": [
      "ASA-201805-25",
      "ASA-201805-24",
      "ASA-201805-22",
      "ASA-201805-23"
    ]
  }
Based on this can you tell me what should be the final state of database records that should exist once this has been processed?

This will help me understand your approach.

Summarizing all the loops(in the extract_vulnerabilities method)for the above given sample JSON feed.

Step-1 Initially, we took a package Pi from the packages list and created an empty list package_vulnerabilities.

Step-2 Then we appended all the CVEs one by one corresponding to affected_version and Pi in the package_vulnerabilities list, something like shown below:

[
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11362',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11361',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	}
	.
	.
	.
]

Step-3 Then we appended all the CVEs one by one corresponding to fixed_version and Pi in the package_vulnerabilities list, something like shown below:

[
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11362',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11361',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	}
	.
	.
	.
]

Step-4 Repeated step-2 and step-3 with all the package in the packages list.

Finally this entire list of dict is yield back to the caller. I am attatching the entire list which will be yielded for the above sample JSON data in this file for reference.

pombredanne · 2019-03-13T14:20:34Z

@lohani2280 please do not link to Gdocs in tickets or PR. I did not open that link. Attach it to the ticket or paste it in the ticket.

pombredanne · 2019-03-13T14:24:41Z

Note: I care only to see the end results at this stage... not the intermediate steps to get there. I think there are issues with your approach and rather than trying to unwind it, it is easier for me to comment on the final results for a single record.

lohani2280 · 2019-03-14T14:17:53Z

@pombredanne Following is the final set of created records for the snippet you provided as an example in above comment:

[
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11362',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11361',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11360',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11359',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11358',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11357',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11356',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11355',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11354',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11362',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11361',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11360',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11359',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11358',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11357',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11356',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11355',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-commons',
	    'vulnerability_id': 'CVE-2018-11354',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11362',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11361',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11360',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11359',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11358',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11357',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11356',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11355',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11354',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11362',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11361',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11360',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11359',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11358',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11357',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11356',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11355',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-gtk',
	    'vulnerability_id': 'CVE-2018-11354',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
		{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11362',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11361',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11360',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11359',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11358',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11357',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11356',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11355',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11354',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11362',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11361',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11360',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11359',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11358',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11357',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11356',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11355',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-cli',
	    'vulnerability_id': 'CVE-2018-11354',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
		{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11362',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11361',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11360',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11359',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11358',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11357',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11356',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11355',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11354',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.0-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11362',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11361',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11360',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11359',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11358',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11357',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11356',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11355',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
	{
	    'package_name': 'wireshark-qt',
	    'vulnerability_id': 'CVE-2018-11354',
	    'description': 'multiple issues',
	    'status': 'Fixed',
	    'severity': 'Critical',
	    'version': '2.6.1-1'
	},
]

pombredanne · 2019-03-14T14:53:55Z

OK, these are 72 records...but I am interested in what will end up in the models.
Do you end up with something more or less like this?

There are four packages:

       "wireshark-common",
       "wireshark-gtk",
       "wireshark-cli",
       "wireshark-qt"

and each comes with two versions:

    "affected": "2.6.0-1",
    "fixed": "2.6.1-1",

So that eight Package records each with one Package reference that point to actual ArchLinux packages

Then we have
one group:

    "name": "AVG-708",

nine CVEs

    "issues": [
      "CVE-2018-11362",
      "CVE-2018-11361",
      "CVE-2018-11360",
      "CVE-2018-11359",
      "CVE-2018-11358",
      "CVE-2018-11357",
      "CVE-2018-11356",
      "CVE-2018-11355",
      "CVE-2018-11354"

and four advisories

    "advisories": [
      "ASA-201805-25",
      "ASA-201805-24",
      "ASA-201805-22",
      "ASA-201805-23"

Somehow I would expect one single Vulnerability record for all these. And many (14) vulnerability references each with a proper id and URL.

Then for each of the affected and fixed, there should be relationships established between packages and vulnerabilities.
So I would expect 4 x 1 relationships for affected and 4 x 1 relationships for fixed.

This is what you need to document and specify so I can review this.

Once you have this, this can become your expected test for an end state and you can work to make your code pass this expected test.

lohani2280 · 2019-03-17T12:50:10Z

@pombredanne
I have tried incorporating the things as you suggested in your last comment. Please have a look and suggest ways to improve this.
Now, the final set of created records for the snippet you provided as an example in above comment is:

{
    'packages_name':[
      'wireshark-common',
      'wireshark-gtk',
      'wireshark-cli',
      'wireshark-qt'
    ],
    'vulnerability_id': [      
      'CVE-2018-11362',
      'CVE-2018-11361',
      'CVE-2018-11360',
      'CVE-2018-11359',
      'CVE-2018-11358',
      'CVE-2018-11357',
      'CVE-2018-11356',
      'CVE-2018-11355',
      'CVE-2018-11354',
      'ASA-201805-25',
      'ASA-201805-24',
      'ASA-201805-22',
      'ASA-201805-23',
      'AVG-708'
    ], 
    'description': 'multiple issues',
    'status': 'Fixed', 
    'severity': 'Critical', 
    'version': ['2.6.0-1', '2.6.1-1']}
}

Also, this is what will end up in the model for the sample record taken fom arch linux issue tracker:

eight Package records(with package name and version) each with one Package reference that point to actual ArchLinux packages
one single Vulnerability record with a description
many (14) vulnerability references each with a proper id
4 x 1 relationships for affected version through ImpactedPackage and 4 x 1 relationships for fixed version through ResolvedPackage

pombredanne

Thanks! the code is clean (see my minor comments), and your explanation makes sense. But you removed all the tests? I would like to see the results of a test that uses the wireshark example and asserts the end results in the DB.

pombredanne · 2019-03-19T06:28:08Z

+
+def scrape_vulnerabilities():
+    """
+    Scrape arch linux' security tracker.


You should tell what this function returns instead.
Return ...

pombredanne · 2019-03-19T06:28:12Z

+
+def extract_vulnerabilities(arch_data):
+    """
+    Return a series of mappings for each record of arch linux security tracker


series is not a very common name for an iterable. Either use Yield mappings ... or Return an iterable or ...

pombredanne · 2019-03-19T06:32:01Z

+        package_vulnerabilities = []
+        cves = item['issues']
+        advisories = set(item['advisories'])
+        vulnerabilities = cves + list(advisories)


In mixing it all in a list, you are losing their origin and will have to parser these back afterwards to craft a proper URL for each.
Also what is the item['name'] below?
The end results is to get these as references, right? then you should compute the fields that are useful and needed for this now.

@pombredanne No. For crafting a proper URL for each I don't need their origin as the URL doesn't depend on origin. Its available directly as https://security.archlinux.org/vulnerability_id
Example :
https://security.archlinux.org/CVE-2018-20592
https://security.archlinux.org/AVG-922
https://security.archlinux.org/ASA-201811-20

For https://security.archlinux.org/CVE-2018-20592 would not there be also a URL such as https://nvd.nist.gov/vuln/detail/CVE-2018-20592 ? (which means that these are likely two references)

Yes. Infact there are even more than 2 references available here https://security.archlinux.org/CVE-2018-20592 as you mentioned #33 (comment).

pombredanne · 2019-03-19T06:33:40Z

+            'description': item['type'],
+            'status': item['status'],
+            'severity': item['severity'],
+            'version': [affected_version,fixed_version]


why use a positional list rather than two named items?

pombredanne · 2019-03-19T06:34:45Z

+    for data in extract_data:
+        packages_name = data['packages_name']
+        vulnerabilities = data['vulnerability_id']
+        affected_version = data['version'][0]


Do not use this style of positional items. Use two items instead

pombredanne · 2019-03-19T06:36:15Z

+        affected_version = item['affected']
+        fixed_version = item['fixed']
+        if not fixed_version:
+            fixed_version = 'None'


Never use a 'None' string. Use a plain None

@pombredanne I tried but if I use plain None, I get following error while dumping into db:
sqlite3.IntegrityError: NOT NULL constraint failed: vulnerabilities_package.version

you will need to show some actual error message so I can understand the issue. But imho this issue will likely vanish after the other refactorings

pombredanne · 2019-03-19T06:38:46Z

+        if not fixed_version:
+            fixed_version = 'None'
+
+        package_vulnerabilities = {


What is this data structure bringing as value on top of the original data structure?
Either you craft something that adapts and transforms the data to the model for easy import and precomputes fields or may be it would be simpler to use the Arch JSON data directly to create models?

I reckon that you are using the same approach as was used before, but that approach may not be correct. This is ok to question and change

@pombredanne I agree that package_vulnerability data structure doesn't bring any direct value on top of the original data structure. But IMO its not good on part of users to use the Arch JSON data directly to create models. I mean the users might want to view/observe the data before actually dumping it into the db. I'd rather suggest that instead of having 2 methods namely extract_vulnerabilities and scrape_vulnerabilities we could have only 1 of the method like extract_vulnerabilities. Also, we should leave archlinux_dump as it is. Please suggest your views on this.

I mean the users might want to view/observe the data before actually dumping it into the db.

that's not the goal: we want to import fully automatically... the data will never ever be reviewed before it is imported

so the only reason why you want to a tow step approach is for when the data is not structured in the first place. Here this is a JSON dump so there is not much of a transformation needed in a structure format. The key is converting the data to proper DB records in the simplest way. Does this make sense to you?

pombredanne · 2019-03-19T07:32:19Z

+            VulnerabilityReference.objects.create(
+                vulnerability=vulnerability,
+                reference_id=vulnerability_id,
+                source='archlinux',


Not all is from archlinux... CVEs are not from there.
And we should have a URL for each https://github.com/nexB/vulnerablecode/blob/55a633db3c09d8b78d21bffea25a510e1444acdf/vulnerabilities/models.py#L48

pombredanne · 2019-03-25T19:13:01Z

+            PackageReference.objects.create(
+                package=package_fixed,
+                repository='https://security.archlinux.org/package/'+
+                            '%s'% package_name


BTW, why

'https://security.archlinux.org/package/'+ '%s'% package_name

and not
'https://security.archlinux.org/package/%s' % package_name
both here and above?

pombredanne · 2019-03-25T19:21:12Z

Also I wonder how to get all the vulnerability references that are listed on this https://security.archlinux.org/CVE-2018-20592 ?

pombredanne · 2019-03-25T19:21:48Z

Also please check the CI tests failures: https://travis-ci.org/nexB/vulnerablecode/builds/507496621?utm_source=github_status&utm_medium=notification

lohani2280 · 2019-04-01T14:52:35Z

@pombredanne

Also I wonder how to get all the vulnerability references that are listed on this https://security.archlinux.org/CVE-2018-20592 ?

I can crawl and extract all those vulnerability references from https://security.archlinux.org/CVE-2018-20592 using python. But then we need to modify our models to store multiple vulnerability references.

singh1114 · 2019-04-10T16:44:28Z

 #  VulnerableCode is a free software code scanning tool from nexB Inc. and others.
 #  Visit https://github.com/nexB/vulnerablecode/ for support and download.

+


No need of this extra line.

singh1114 · 2019-04-10T16:46:10Z

+      "ASA-201805-23"
+    ]
+  }
+]


Keep an extra blank line in each file.

singh1114 · 2019-04-10T16:47:52Z

 from vulnerabilities.scraper import debian
 from vulnerabilities.scraper import ubuntu

-


Two blank lines after import statements.

Signed-off-by: lohani2280 <lohani.ayush01@gmail.com>

pombredanne

LGTM! there are a few things that could still be refined, but that quite mergeable now! thank you ++ for working through this

Signed-off-by: Jono Yang <jyang@nexb.com>

* Create copyright statement from holder information Signed-off-by: Jono Yang <jyang@nexb.com>

* This is used for the case where we are starting off a project and have not yet generated requirements files Signed-off-by: Jono Yang <jyang@nexb.com>

Signed-off-by: Jono Yang <jyang@nexb.com>

* Add --init option to configure.bat * Update help text in configure and configure.bat Signed-off-by: Jono Yang <jyang@nexb.com>

Signed-off-by: Jono Yang <jyang@nexb.com>

* Update README.rst Signed-off-by: Jono Yang <jyang@nexb.com>

* Update README.rst with instructions for post-initialization usage Signed-off-by: Jono Yang <jyang@nexb.com>

* Replace references to scancode-toolkit repo with links to the skeleton repo * Remove --python option from configure.bat Signed-off-by: Jono Yang <jyang@nexb.com>

lohani2280 force-pushed the test_branch_1 branch 2 times, most recently from a2645c3 to 56a5a6c Compare February 28, 2019 04:49

pombredanne requested changes Feb 28, 2019

View reviewed changes

lohani2280 force-pushed the test_branch_1 branch from 56a5a6c to 6e22217 Compare March 3, 2019 09:07

lohani2280 changed the title ~~Collected vulnerabilities from arch linux~~ Collect vulnerabilities from arch linux Mar 3, 2019

pombredanne requested changes Mar 6, 2019

View reviewed changes

lohani2280 force-pushed the test_branch_1 branch from 6e22217 to 5d6c681 Compare March 10, 2019 08:15

pombredanne requested changes Mar 11, 2019

View reviewed changes

lohani2280 force-pushed the test_branch_1 branch from 5d6c681 to 6b4ddf1 Compare March 12, 2019 17:13

pombredanne requested changes Mar 13, 2019

View reviewed changes

lohani2280 force-pushed the test_branch_1 branch from 6b4ddf1 to 65ca798 Compare March 17, 2019 12:47

lohani2280 force-pushed the test_branch_1 branch 3 times, most recently from 5190572 to 4748733 Compare March 17, 2019 15:08

pombredanne requested changes Mar 19, 2019

View reviewed changes

pombredanne reviewed Mar 25, 2019

View reviewed changes

lohani2280 force-pushed the test_branch_1 branch 2 times, most recently from 0d662c3 to 8daf1df Compare April 1, 2019 14:48

singh1114 suggested changes Apr 1, 2019

View reviewed changes

Comment thread vulnerabilities/data_dump.py Outdated

Comment thread vulnerabilities/scraper/archlinux.py

Comment thread vulnerabilities/tests/test_scrapers.py Outdated

lohani2280 force-pushed the test_branch_1 branch 2 times, most recently from a67e87e to 5acdd10 Compare April 9, 2019 21:44

singh1114 reviewed Apr 10, 2019

View reviewed changes

Comment thread vulnerabilities/data_dump.py Outdated

singh1114 reviewed Apr 10, 2019

View reviewed changes

Comment thread vulnerabilities/data_dump.py

lohani2280 force-pushed the test_branch_1 branch from 5acdd10 to 5989e5c Compare April 10, 2019 07:12

singh1114 reviewed Apr 10, 2019

View reviewed changes

lohani2280 force-pushed the test_branch_1 branch 4 times, most recently from 81ad043 to 8b49dfc Compare April 11, 2019 16:24

lohani2280 added 3 commits April 11, 2019 22:02

Collect vulnerabilities from arch linux aboutcode-org#20

0f124c1

Signed-off-by: lohani2280 <lohani.ayush01@gmail.com>

Fix tests

ca64624

Signed-off-by: lohani2280 <lohani.ayush01@gmail.com>

Update README on how to add archlinux data into the database.

43a35ce

Signed-off-by: lohani2280 <lohani.ayush01@gmail.com>

lohani2280 force-pushed the test_branch_1 branch from 8b49dfc to 43a35ce Compare April 11, 2019 16:49

singh1114 approved these changes Apr 11, 2019

View reviewed changes

pombredanne approved these changes Apr 11, 2019

View reviewed changes

pombredanne merged commit 7343bde into aboutcode-org:develop Apr 11, 2019

pombredanne mentioned this pull request Apr 11, 2019

Collect vulnerabilities from arch linux #20

Closed

pombredanne pushed a commit that referenced this pull request Apr 2, 2025

Add scripts from scancode-toolkit/etc/release/ #33

e9067c8

Signed-off-by: Jono Yang <jyang@nexb.com>

pombredanne pushed a commit that referenced this pull request Apr 2, 2025

Normalize license in load_pkginfo_data #33

0e1f56b

* Create copyright statement from holder information Signed-off-by: Jono Yang <jyang@nexb.com>

pombredanne pushed a commit that referenced this pull request Apr 2, 2025

Add --init option to configure #33

288532d

* This is used for the case where we are starting off a project and have not yet generated requirements files Signed-off-by: Jono Yang <jyang@nexb.com>

pombredanne pushed a commit that referenced this pull request Apr 2, 2025

Update README.rst #33

a5ae4f3

Signed-off-by: Jono Yang <jyang@nexb.com>

pombredanne pushed a commit that referenced this pull request Apr 2, 2025

Update configure.bat #33

9342bc1

* Add --init option to configure.bat * Update help text in configure and configure.bat Signed-off-by: Jono Yang <jyang@nexb.com>

pombredanne pushed a commit that referenced this pull request Apr 2, 2025

Add placeholder requirements.txt files #33

45e4a2a

Signed-off-by: Jono Yang <jyang@nexb.com>

pombredanne pushed a commit that referenced this pull request Apr 2, 2025

Handle multiple options in configure #33

944fbae

Signed-off-by: Jono Yang <jyang@nexb.com>

pombredanne pushed a commit that referenced this pull request Apr 2, 2025

Fix path to aboutcode in utils_thirdparty.py #33

3532b22

* Update README.rst Signed-off-by: Jono Yang <jyang@nexb.com>

pombredanne pushed a commit that referenced this pull request Apr 2, 2025

Handle ExpressionParseError #33

ebcfb93

* Update README.rst with instructions for post-initialization usage Signed-off-by: Jono Yang <jyang@nexb.com>

pombredanne pushed a commit that referenced this pull request Apr 2, 2025

Address review comments #33

bfdc6ff

* Replace references to scancode-toolkit repo with links to the skeleton repo * Remove --python option from configure.bat Signed-off-by: Jono Yang <jyang@nexb.com>



		ARCHLINUX_TRACKER_URL = 'https://security.archlinux.org/json'

		# VulnerableCode is a free software code scanning tool from nexB Inc. and others.
		# Visit https://github.com/nexB/vulnerablecode/ for support and download.

		from vulnerabilities.scraper import debian
		from vulnerabilities.scraper import ubuntu

Uh oh!

Uh oh!

Conversation

lohani2280 commented Feb 28, 2019

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

lohani2280 commented Feb 28, 2019

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lohani2280 commented Mar 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lohani2280 Mar 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lohani2280 Mar 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lohani2280 commented Mar 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

lohani2280 commented Mar 10, 2019 •

edited

Loading

lohani2280 Mar 12, 2019 •

edited

Loading

lohani2280 Mar 12, 2019 •

edited

Loading

lohani2280 commented Mar 13, 2019 •

edited

Loading

lohani2280 commented Mar 17, 2019 •

edited

Loading

lohani2280 Mar 22, 2019 •

edited

Loading