Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for CWE #782

Merged
merged 2 commits into from
Jan 24, 2023
Merged

Add support for CWE #782

merged 2 commits into from
Jan 24, 2023

Conversation

ziadhany
Copy link
Collaborator

Reference: #651
Signed-off-by: Ziad [email protected]

@ziadhany ziadhany marked this pull request as ready for review June 27, 2022 18:15
@ziadhany
Copy link
Collaborator Author

Screenshot from 2022-06-27 20-19-05

@ziadhany ziadhany self-assigned this Jul 5, 2022
@TG1999 TG1999 requested a review from pombredanne July 19, 2022 15:25
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would need to be rebased... also you may want to remove the UI part to treat it separately as the UI has changed extensively!
Also could we also get the CWE from the NVD importer? or is this something for later?
Finally two other considerations:

  1. the CWE library maintainer has not much replied to data, so if we need this we would have to fok it
  2. is there something else beyond CWE that would be about a more general concept of categories?

vulnerabilities/models.py Outdated Show resolved Hide resolved
@ziadhany
Copy link
Collaborator Author

This would need to be rebased... also you may want to remove the UI part to treat it separately as the UI has changed extensively! Also could we also get the CWE from the NVD importer? or is this something for later?

ok, no problem I will change this, but what about editing all importers not just NVD, I think I could handle this in a separate pull request.

1. the CWE library maintainer has not much replied to data, so if we need this we would have to fok it

oops, yes we can fork it. it isn't a complicated library, all data come from this database, we need to make sure the database is updated so I think we should use this link
https://cwe.mitre.org/data/downloads.html as our database.

2. is there something else beyond CWE that would be about a more general concept of categories? 

I don't know but the main three categories are

  • Software Development
  • Hardware Design
  • Research Concepts

some External Mappings :

  • CWE Top 25 (2022)
  • OWASP Top Ten (2021)
  • Software Fault Pattern Clusters

@ziadhany ziadhany force-pushed the add-cwe branch 2 times, most recently from 89ff4d0 to 24a919b Compare September 10, 2022 23:35
@ziadhany
Copy link
Collaborator Author

Screenshot from 2022-09-11 01-38-04

@ziadhany ziadhany mentioned this pull request Sep 22, 2022
9 tasks
@TG1999 TG1999 added this to the v31.0 milestone Oct 11, 2022
@TG1999 TG1999 marked this pull request as draft October 18, 2022 16:26
@ziadhany
Copy link
Collaborator Author

ziadhany commented Nov 1, 2022

image

@pombredanne
Copy link
Member

@ziadhany the cwe2 library is ready now: https://pypi.org/project/cwe2/

<div class="tab-nested-div">
<table class="table is-bordered is-striped is-narrow is-hoverable is-fullwidth gray-header-border">
<tr>
<th> CWE id </th>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use plain CWE

@pombredanne pombredanne modified the milestones: v31.0, v32.0.0 Dec 8, 2022
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here are a few nitpickings for your review

"""
weaknesses = []
for weaknesses in get_item(self.cve_item, "cve", "problemtype", "problemtype_data") or []:
weaknesses_description = weaknesses.get("description")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
weaknesses_description = weaknesses.get("description")
weaknesses_description = weaknesses.get("description") or []

for weaknesses in get_item(self.cve_item, "cve", "problemtype", "problemtype_data") or []:
weaknesses_description = weaknesses.get("description")
for weaknesses_value in weaknesses_description:
cwe_id = get_cwe_id(weaknesses_value.get("value"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would skip weaknesses_description if the lang is not "en"

@@ -133,6 +134,10 @@ def process_inferences(inferences: List[Inference], advisory: Advisory, improver
fix=True,
).update_or_create()

if inference.weaknesses:
for cwe_id in inference.weaknesses:
Weakness.objects.update_or_create(cwe_id=cwe_id, vulnerabilities=vulnerability)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is vulnerabilities plural and receiving a single vulnerability?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think because we improve vulnerability one by one . Can you suggest any way to do this?

@property
def name(self):
"""Return the weakness's name."""
db = Database()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a class member instead

@@ -249,6 +250,25 @@ def get_related_purls(self):
return [p.package_url for p in self.packages.distinct().all()]


class Weakness(models.Model):
cwe_id = models.IntegerField()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a help text

vulnerabilities/models.py Show resolved Hide resolved
@ziadhany ziadhany force-pushed the add-cwe branch 3 times, most recently from 52b1aeb to 3ddaccf Compare December 20, 2022 15:10
@pombredanne
Copy link
Member

@ziadhany Could you rebase or merge on the latest main branch?

Remove the weaknesses table header

Fix expected_files tests

Fix all test cases

Add CWE support for NVD importer

Fix migration conflict

Add Weakness model

Fix requirements.txt , Fix migration conflict

Add cwe name instead of Hyperlinks

Add nexB/cwe package

Fix test , remove empty lines

Add CWE in the new UI

Signed-off-by: ziadhany <[email protected]>
Signed-off-by: ziadhany <[email protected]>
Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@pombredanne pombredanne merged commit 59fd972 into aboutcode-org:main Jan 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants