Handle vulnerabilities which don't have any vulnerability ids #259

sbs2001 · 2020-09-26T06:16:45Z

Fixes #232

Get the Vulnerability model and it's methods right. Use a timestamp as a custom id for now.
Refactor all the codebase to comply to the new Vulnerability model.
Create a repo which would contain all vulnerabilities with custom ids.
Add option to VulnerableCode to either assign these ids or sync the ids from the repo.
Automate dumping the vulnerabilities with custom ids to the repo
Decide the custom id and implement a data structure to make it feasible.

Signed-off-by: Shivam Sandbhor shivam.sandbhor@gmail.com

sbs2001 · 2020-09-26T06:29:11Z

For the models the requirements as we decided upon at #232 are (this is a partial repaste of @pombredanne 's comment)

a. It has a CVE, then we reuse the CVE-xxxx as its identifier
b. It does not have a CVE, then we create the VULCODE-xxxx as its identifier
c. later if that VULCODE-xxx vulnerability gets a CVE, we will replace the id with the CVE-xxx id and move the VULCODE-xxx id as a reference for that vulnerability.

From a usage standpoint, this means that we should be able to search a vulnerability not only based on its identifier (that may change over time) but also based on its references.

Alternatively to c. above we could have a dedicated field to store the previous VULCODE-xxx when this is replaced by an assigned CVE-xxxid.

The model could look like this

Vulnerability
- identifier: VULCODE-xxx or CVE-xxx
- vc_identifier: empty or VULCODE-xxx if identifier is a CVE-xxx

sbs2001 · 2020-09-26T06:35:02Z

The current models gives me this

In [1]: from vulnerabilities import models                                                                                                                                                                           

In [2]: z = models.Vulnerability.objects.create(summary="tar ball")                                                                                                                                                  

In [3]: z.identifier                                                                                                                                                                                                 
Out[3]: '2020-09-26 06:32:18.832135'

sbs2001 · 2020-09-26T08:33:58Z

I'm using https://github.com/sbs2001/vulcodes as the repo for dev purpose.

pombredanne · 2020-09-28T10:29:10Z

I am wondering ..."VULCODE-XXX" feels a little bit too generic, and I am kinda warming up to a the explicit albeit longer "VULNERABLECODE-XXX" prefix even if a tad long... it is explicit and meant to be replaced eventually by a CVE (and it is also very clear where it is coming from)?

pombredanne

Thanks!
Looking good except for the "vulcode/VULCODE" name that I would like to discuss more.

vulnerabilities/management/commands/push.py

pombredanne · 2020-09-28T10:35:28Z

The model could look like this

Vulnerability
    identifier: VULCODE-xxx or CVE-xxx
    vc_identifier: empty or VULCODE-xxx if identifier is a CVE-xxx

I wonder if we should not instead just treat the "past" vc_identifier as just an external reference?

pombredanne · 2020-11-18T10:51:18Z

I revisited https://cve.mitre.org/data/refs/index.html and I suggest this instead prefixVULCOID as a prefix which is rather unique and would be short for Vulnerable Code Id.
We could have ids in this form then where the left part is an ISO-like time stamp e.g. VULCOID-YYYY-MM-DD-HH-MM-SS
The id would case insensitive with an uppercase canonical form.
Would this work?

sbs2001 · 2020-11-27T13:49:50Z

@pombredanne VULCOID-YYYY-MM-DD-HH-MM-SS works, have you considered VULCOID-YYYY-<incremental id> ie something like CVE . FWIW It's easier to remember than the timestamp thus easy to reference.

pombredanne · 2020-11-27T14:30:02Z

@sbs2001 re #259 (comment)

VULCOID-YYYY-MM-DD-HH-MM-SS works, have you considered VULCOID-YYYY-<incremental id> ie something like CVE . FWIW It's easier to remember than the timestamp thus easy to reference.

incremental id works too but requires more coordination than a context free timestamp. I am not sure a sequential number is easier to remember than a time stamp though it can be shorter.
We could go with:
VULCOID-YYYY-MM-DD-<incremental id> as a compromise?

sbs2001 · 2020-11-29T10:50:43Z

@pombredanne

incremental id works too but requires more coordination than a context free timestamp

How so ?

The timestamp is dependent on when the instance encountered the vulnerability. coordination is needed in that case too if we want 2 instances to "have a common language" .

IMHO coordination is unavoidable.

pombredanne · 2020-12-11T12:14:38Z

IMHO VULCOID-YYYY-MM-DD-HH-MM-SS is the scheme that requires no coordination and creates the least risk of collision and is still somewhat readable by a human. If there are collision across DBs that's OK.

sbs2001 · 2020-12-17T06:51:09Z

@pombredanne It made sense to me about using timestamps instead of incremental ids after the chat. Main reason being, when using incremental id's it is guaranteed that collisions would occur ( between different instances) and hence they would then be needed to be resolved. This won't happen when using timestamps.

sbs2001 · 2021-02-05T08:05:20Z

@pombredanne I am stashing the code to extract vulcoids to some other branch. This PR won't add that, that feature seems pre-mature to me.

pombredanne

Thanks! See my comments inline. IMHO the main issue we have is the name "identifier"
Either we use that everywhere, of may be we use vulnerability_id instead which would be more explicit and work in all the contexts.

vulnerabilities/api.py

vulnerabilities/data_source.py

vulnerabilities/import_runner.py

vulnerabilities/importer_yielder.py

vulnerabilities/importers/safety_db.py

vulnerabilities/migrations/0001_initial.py

vulnerabilities/models.py

vulnerabilities/api.py

vulnerabilities/models.py

pombredanne · 2021-02-23T10:46:49Z

vulnerabilities/importers/safety_db.py

+
            all_package_versions = self.versions.get(package_name)
-            if len(all_package_versions) == 0:
+            if not len(all_package_versions):


Suggested change

if not len(all_package_versions):

if not all_package_versions:

Doing the safetydb changes in other pr

pombredanne · 2021-02-23T10:47:00Z

vulnerabilities/importers/safety_db.py

-                # meaning if cve_ids is not [''] but either ['CVE-123'] or ['CVE-123, CVE-124']
-                if len(cve_ids[0]):
-                    cve_ids = [s.strip() for s in cve_ids.split(",")]
+                if advisory["cve"]:


May be create a variable

Doing the safetydb changes in other pr

vulnerabilities/models.py

pombredanne

All good ... I have a few minor nit pickings but this is good to merge!
Thanks!

This reduces the dependence on CVE ID. Cases where vulnerability don't have CVE can be handled Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

This command takes as input a remote repo's url. Upon invoking the command all the vulnerabilities which were id'd by vulnerablecode will be pushed to this repo. Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

* Added incremental time id in import_runner.py to prevent vulnerability id conflicts Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

* In model Vulnerability "identifier" -> "vulnerability_id" * In Advisory dataclass "identifier" -> "vulnerability_id" Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

sbs2001 force-pushed the custom_vuln_ids branch from 43bb7a7 to 3538474 Compare September 26, 2020 08:27

sbs2001 force-pushed the custom_vuln_ids branch 2 times, most recently from 705b859 to 020788c Compare September 26, 2020 13:26

pombredanne reviewed Sep 28, 2020

View reviewed changes

vulnerabilities/management/commands/push.py Outdated Show resolved Hide resolved

sbs2001 changed the title ~~[WIP] Handle vulnerabilities which don't have any vulnerability ids~~ Handle vulnerabilities which don't have any vulnerability ids Oct 7, 2020

pombredanne mentioned this pull request Nov 18, 2020

On the identification of vulnerabilities #232

Closed

sbs2001 force-pushed the custom_vuln_ids branch from 5e1547a to be26c53 Compare December 17, 2020 10:02

sbs2001 mentioned this pull request Dec 19, 2020

REST API: Bulk requests for packages and vulnerabilities #284

Closed

sbs2001 force-pushed the custom_vuln_ids branch from be26c53 to 38eee2b Compare December 19, 2020 05:55

pombredanne mentioned this pull request Feb 2, 2021

Add django admin functionality for searching and filtering objects #330

Merged

sbs2001 force-pushed the custom_vuln_ids branch from 38eee2b to 3d710ea Compare February 3, 2021 09:48

sbs2001 force-pushed the custom_vuln_ids branch from b51b430 to b8fa296 Compare February 5, 2021 12:04

pombredanne requested changes Feb 9, 2021

View reviewed changes

sbs2001 force-pushed the custom_vuln_ids branch from 4b1fc9f to 48750ca Compare February 10, 2021 05:36

pombredanne reviewed Feb 10, 2021

View reviewed changes

vulnerabilities/migrations/0001_initial.py Outdated Show resolved Hide resolved

pombredanne reviewed Feb 11, 2021

View reviewed changes

vulnerabilities/models.py Show resolved Hide resolved

pombredanne reviewed Feb 11, 2021

View reviewed changes

vulnerabilities/models.py Outdated Show resolved Hide resolved

tdruez requested changes Feb 12, 2021

View reviewed changes

vulnerabilities/api.py Outdated Show resolved Hide resolved

vulnerabilities/models.py Outdated Show resolved Hide resolved

pombredanne reviewed Feb 23, 2021

View reviewed changes

vulnerabilities/models.py Outdated Show resolved Hide resolved

pombredanne reviewed Feb 23, 2021

View reviewed changes

vulnerabilities/models.py Show resolved Hide resolved

pombredanne approved these changes Feb 23, 2021

View reviewed changes

sbs2001 added 21 commits February 23, 2021 17:53

⚡ Change Vulnerability model to use custom id

b38f783

This reduces the dependence on CVE ID. Cases where vulnerability don't have CVE can be handled Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

🔨 Refactor codebase to use new Vulnerability model

6358d3d

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

🎉 Add push management command

a796bc9

This command takes as input a remote repo's url. Upon invoking the command all the vulnerabilities which were id'd by vulnerablecode will be pushed to this repo. Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

🏗️ Add option to whether create custom vulcodes or not

d7badc6

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

➕ Add a VulCode importer

1c649ea

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Sync with main branch and fix latest tests.

c944101

* Added incremental time id in import_runner.py to prevent vulnerability id conflicts Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Rebase to latest main

2f2a02e

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Remove the push command

fe47444

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Remove the importer for vulcodes

3bbbd62

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Remove vulcode creation option

0e0ac77

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Fix tests for import_runner

c475323

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Disabble safetydb importer

ea03013

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Add comments to explain logic of assigning vulcoids

fa84677

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Change field names in Vulnerability Model and Advisory dataclass

2ffbfbb

* In model Vulnerability "identifier" -> "vulnerability_id" * In Advisory dataclass "identifier" -> "vulnerability_id" Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Update migration script

d74603e

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Move VULCOID generation to Vulnerability model

ea267f3

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Add tests for Vulnerability model's save method

46b21c8

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Use microsecond in vulcoids

0ab075b

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Rebase and resolve confilcts

d93887e

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Use full microsecond in VULCOID

e94d2a0

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

Make review changes (Final Polish ;) )

e48fa44

Signed-off-by: Shivam Sandbhor <shivam.sandbhor@gmail.com>

sbs2001 force-pushed the custom_vuln_ids branch from c43a81c to e48fa44 Compare February 23, 2021 12:24

sbs2001 merged commit 9fe5864 into aboutcode-org:main Feb 23, 2021

This was referenced Feb 23, 2021

Change data models, to fix existing issues #206

Closed

Fix and renable safetydb importer #273

Closed

	if not len(all_package_versions):
	if not all_package_versions:

Uh oh!

Handle vulnerabilities which don't have any vulnerability ids #259

Handle vulnerabilities which don't have any vulnerability ids #259

Uh oh!

Conversation

sbs2001 commented Sep 26, 2020 • edited by pombredanne Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbs2001 commented Sep 26, 2020

Uh oh!

sbs2001 commented Sep 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbs2001 commented Sep 26, 2020

Uh oh!

pombredanne commented Sep 28, 2020

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pombredanne commented Sep 28, 2020

Uh oh!

pombredanne commented Nov 18, 2020

Uh oh!

sbs2001 commented Nov 27, 2020

Uh oh!

pombredanne commented Nov 27, 2020

Uh oh!

sbs2001 commented Nov 29, 2020

Uh oh!

pombredanne commented Dec 11, 2020

Uh oh!

sbs2001 commented Dec 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sbs2001 commented Feb 5, 2021

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pombredanne Feb 23, 2021

Choose a reason for hiding this comment

Uh oh!

sbs2001 Feb 23, 2021

Choose a reason for hiding this comment

Uh oh!

pombredanne Feb 23, 2021

Choose a reason for hiding this comment

Uh oh!

sbs2001 Feb 23, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sbs2001 commented Sep 26, 2020 •

edited by pombredanne

Loading

sbs2001 commented Sep 26, 2020 •

edited

Loading

sbs2001 commented Dec 17, 2020 •

edited

Loading