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 [email protected]

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

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 <[email protected]>

Signed-off-by: Shivam Sandbhor <[email protected]>

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 <[email protected]>

Signed-off-by: Shivam Sandbhor <[email protected]>

* Added incremental time id in import_runner.py to prevent vulnerability id conflicts Signed-off-by: Shivam Sandbhor <[email protected]>

Signed-off-by: Shivam Sandbhor <[email protected]>

* In model Vulnerability "identifier" -> "vulnerability_id" * In Advisory dataclass "identifier" -> "vulnerability_id" Signed-off-by: Shivam Sandbhor <[email protected]>

Signed-off-by: Shivam Sandbhor <[email protected]>

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

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 <[email protected]>

🔨 Refactor codebase to use new Vulnerability model

6358d3d

Signed-off-by: Shivam Sandbhor <[email protected]>

🎉 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 <[email protected]>

🏗️ Add option to whether create custom vulcodes or not

d7badc6

Signed-off-by: Shivam Sandbhor <[email protected]>

➕ Add a VulCode importer

1c649ea

Signed-off-by: Shivam Sandbhor <[email protected]>

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 <[email protected]>

Rebase to latest main

2f2a02e

Signed-off-by: Shivam Sandbhor <[email protected]>

Remove the push command

fe47444

Signed-off-by: Shivam Sandbhor <[email protected]>

Remove the importer for vulcodes

3bbbd62

Signed-off-by: Shivam Sandbhor <[email protected]>

Remove vulcode creation option

0e0ac77

Signed-off-by: Shivam Sandbhor <[email protected]>

Fix tests for import_runner

c475323

Signed-off-by: Shivam Sandbhor <[email protected]>

Disabble safetydb importer

ea03013

Signed-off-by: Shivam Sandbhor <[email protected]>

Add comments to explain logic of assigning vulcoids

fa84677

Signed-off-by: Shivam Sandbhor <[email protected]>

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 <[email protected]>

Update migration script

d74603e

Signed-off-by: Shivam Sandbhor <[email protected]>

Move VULCOID generation to Vulnerability model

ea267f3

Signed-off-by: Shivam Sandbhor <[email protected]>

Add tests for Vulnerability model's save method

46b21c8

Signed-off-by: Shivam Sandbhor <[email protected]>

Use microsecond in vulcoids

0ab075b

Signed-off-by: Shivam Sandbhor <[email protected]>

Rebase and resolve confilcts

d93887e

Signed-off-by: Shivam Sandbhor <[email protected]>

Use full microsecond in VULCOID

e94d2a0

Signed-off-by: Shivam Sandbhor <[email protected]>

Make review changes (Final Polish ;) )

e48fa44

Signed-off-by: Shivam Sandbhor <[email protected]>

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

sbs2001 commented Sep 26, 2020 •

edited by pombredanne

Loading

sbs2001 commented Sep 26, 2020

sbs2001 commented Sep 26, 2020 •

edited

Loading

sbs2001 commented Sep 26, 2020

pombredanne commented Sep 28, 2020

pombredanne left a comment

pombredanne commented Sep 28, 2020

pombredanne commented Nov 18, 2020

sbs2001 commented Nov 27, 2020

pombredanne commented Nov 27, 2020

sbs2001 commented Nov 29, 2020

pombredanne commented Dec 11, 2020

sbs2001 commented Dec 17, 2020 •

edited

Loading

sbs2001 commented Feb 5, 2021

pombredanne left a comment

pombredanne Feb 23, 2021

sbs2001 Feb 23, 2021

pombredanne left a comment

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

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

Conversation

sbs2001 commented Sep 26, 2020 • edited by pombredanne Loading

sbs2001 commented Sep 26, 2020

sbs2001 commented Sep 26, 2020 • edited Loading

sbs2001 commented Sep 26, 2020

pombredanne commented Sep 28, 2020

pombredanne left a comment

Choose a reason for hiding this comment

pombredanne commented Sep 28, 2020

pombredanne commented Nov 18, 2020

sbs2001 commented Nov 27, 2020

pombredanne commented Nov 27, 2020

sbs2001 commented Nov 29, 2020

pombredanne commented Dec 11, 2020

sbs2001 commented Dec 17, 2020 • edited Loading

sbs2001 commented Feb 5, 2021

pombredanne left a comment

Choose a reason for hiding this comment

pombredanne Feb 23, 2021

Choose a reason for hiding this comment

sbs2001 Feb 23, 2021

Choose a reason for hiding this comment

pombredanne left a comment

Choose a reason for hiding this comment

sbs2001 commented Sep 26, 2020 •

edited by pombredanne

Loading

sbs2001 commented Sep 26, 2020 •

edited

Loading

sbs2001 commented Dec 17, 2020 •

edited

Loading