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

Report only those fixed versions that are greater than the affected version #1228

Closed
johnmhoran opened this issue Jul 8, 2023 · 46 comments
Closed

Comments

@johnmhoran
Copy link
Member

The VulnerableCode UI and API currently report a full set of all fixed versions of a package that are related directly or indirectly to a particular affected version, including fixed versions whose version number is less than the affected version number. This seems potentially confusing to users and inaccurately identifies package versions that a user should consider to address an identified vulnerability.

It would be more useful (and accurate) if the VulnerableCode UI and API reported only those fixed versions that are greater than the particular affected version at issue. I've attached a .xlsx file containing examples of fixed-version mismatches.

See also

examples-of-affected-vs-fixed-version-mismatches-v0.01.xlsx

@DennisClark
Copy link
Member

good idea

@johnmhoran johnmhoran self-assigned this Jul 11, 2023
johnmhoran added a commit that referenced this issue Jul 26, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Jul 28, 2023
…ge matching #1228

Reference: #1228

Note that my updated code is still in testing/dev stage and has not yet been completed or cleaned.

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Jul 30, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
@johnmhoran
Copy link
Member Author

My push is still a work-in-progress and retains many print statements for ongoing exploration of the data and functions. The terminal displays some relevant high-level data about the affected package, the vulns and the fixing packages.

Working on the Packages class in models.py, I've created a function get_fixing_packages(self) in which Prefetch takes the list of vulns affecting the current package, retrieves a list of the fixing packages for each vuln, and assigns the result to a custom attribute, filtered_fixed_packages, that we can call when needed.

We also have the very beginnings of a function assign_univers_version(self, fixing_pkg), which identifies which univers version applies to the two packages to be compared (self and a fixing package), evaluates whether the fixing_pkg version is > than the target affected package, and returns True or False. I need to add more univers versions to the dictionary; and still need to check which fixing version is closest to (but greater than) the affected version.

All of this is conveyed to the Jinja2 template (package_details.html) with this context key-value pair in the PackageDetails class in views.py: context["get_fixing_packages"] = package.get_fixing_packages -- called in the Jinja2 template with {% if pkg in get_fixing_packages %}.

I'm using some red and green colors in the Package details template to highlight the test data and have included a short explanation in the template for the time being. These 4 example searches (all maven for now) illustrate the fixing packages we start with (red) and the filtering we do to match type, namespace etc. and discard "backport" versions (green).

pkg:maven/net.minidev/json-smart@2.3
pkg:maven/org.apache.hadoop/hadoop-common@3.2.2
pkg:maven/org.springframework/spring-webmvc@4.1.0
pkg:maven/com.fasterxml.jackson.core/jackson-databind@2.13.1

@johnmhoran
Copy link
Member Author

More to do, including cleaning up the code, updating existing tests and adding a few new tests for the latest work, but this does look cleaner/less cluttered with data. In addition, I've changed all instances of PURL to purl. Is that what we want?

image

@pombredanne
Copy link
Member

Here are some suggestion for your consideration:

  • The "closest fixed by packages" should just be "Fixed by" IMHO. This is simpler and this is the concept exactly anyway, so no need to get "closest". The fact we have purl makes it obvious these are packages. Or we could even go as "Fixed by versions" and only list versions (and link to purls), to be discussed.

  • "Vulnerabilities: 5 ?" may be more explicit as "Other vulnerabilities" or "Affected by 5 other vulnerabilities ?"

  • The text popup behind the ❓ question mark may be better as "This version is affected by these other vulnerabilities:"

  • "Closest non-vulnerable purl" may be better as simply "Non-vulnerable version", and were we would display only the version, yet link to the purl? (the API would get the purl)

  • In the same way, "Latest non-vulnerable purl" may be better as "Latest non-vulnerable version", and were we would display only the version, yet link to the purl? (the API would get the purl too). Also we could consider the latest minor version (here some version 9.x) vs. the latest major version( here some version 10.x) separately, as packages with multiple version lines like Tomcat would benefit from this.

In general I feel avoiding the visual repetition of the purl everywhere when only the version changed would simplify the display.

Also what happens when:

  • there is no other vulnerabilities?
  • there is no package fixing a vulnerability?
  • there is no latest vulnerable?

@pombredanne
Copy link
Member

I've changed all instances of PURL to purl.

I think this is something we need to decide and apply across all projects and docs uniformly. @adaaaam @DennisClark @mjherzog ... what's your take?

  • PURL, purl, Purl, pURL?
  • package URL, Package URL, package-url, Package-URL, package-URL?

@adaaaam
Copy link
Member

adaaaam commented Aug 9, 2023

i thought we previously agreed on Package-URL or PURL... we need to pick one and be consistent going forward.

@DennisClark
Copy link
Member

I agree with Adam on this one:
long-form: Package-URL
short-form: PURL

although, I am open to strong arguments in favor of purl for the short-form.

@mjherzog
Copy link
Member

mjherzog commented Aug 9, 2023

+1 for PURL and Package-URL; "purl" gets lost anytime you see it in a sentence. This is not like Package vs package where the core term is generic out in the world

johnmhoran added a commit that referenced this issue Aug 10, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Aug 10, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
@johnmhoran
Copy link
Member Author

johnmhoran commented Aug 10, 2023

I've just committed and pushed my latest code to the open PR (#1249), with more work to do, starting with trying to replace my helper functions and related code with built-in univers capabilities.

All 7 checks passed, but I see an alert of a conflict in vulnerabilities/tests/test_models.py. Looking at https://github.com/nexB/vulnerablecode/pull/1249/conflicts, it seems the following code has been added at the bottom of my new class TestPackageModel(TestCase).

    def test_cwe_not_present_in_weaknesses_db(self):
        w1 = models.Weakness.objects.create(name="189")
        assert w1.weakness is None
        assert w1.name is ""
        assert w1.description is ""

That looks fine and I suppose I could just OK it using the GH editor, then do a pull from my branch to update the local version.

BUT: what is this and where did it come from? I did not add it, and I see no record in the issue or the PR of some outside addition -- or maybe I'm missing that? 🤔

johnmhoran added a commit that referenced this issue Aug 14, 2023
… tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
@johnmhoran
Copy link
Member Author

Just committed and pushed my latest. More to do including merging main.

johnmhoran added a commit that referenced this issue Aug 15, 2023
Reference: #1228

Signed-off-by: John M. Horan johnmhoran@gmail.com
johnmhoran added a commit that referenced this issue Aug 15, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Aug 15, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
@johnmhoran
Copy link
Member Author

johnmhoran commented Aug 15, 2023

@pombredanne I just did another push of my latest, this time addressing your UI comments from last week:

The "closest fixed by packages" should just be "Fixed by" IMHO. This is simpler and this is the concept exactly anyway, so no need to get "closest". The fact we have purl makes it obvious these are packages. Or we could even go as "Fixed by versions" and only list versions (and link to purls), to be discussed.

  • Agreed -- simpler/cleaner. Changed to "Fixed by packages". (I like the versions suggestions as well.) If you meant "Fixed by" and no reference to "packages", let me know.

"Vulnerabilities: 5 ?" may be more explicit as "Other vulnerabilities" or "Affected by 5 other vulnerabilities ?"

  • Good idea. This now reads "Affected by [number] other vulnerability/vulnerabilities." -- the last term is chosen with a Jinja2 if/else (which also addresses 0 fixed_by and 0 other vulnerabilities permutations).

The text popup behind the ❓ question mark may be better as "This version is affected by these other vulnerabilities:"

  • Agreed and done.

"Closest non-vulnerable purl" may be better as simply "Non-vulnerable version", and were we would display only the version, yet link to the purl? (the API would get the purl)

  • Done. I added version data to the dictionary returned by my new property method fixed_package_details() and refer to the version rather than the purl in the Package details page. Definitely a cleaner/crisper display.

In the same way, "Latest non-vulnerable purl" may be better as "Latest non-vulnerable version", and were we would display only the version, yet link to the purl? (the API would get the purl too).

  • Done.

Also we could consider the latest minor version (here some version 9.x) vs. the latest major version( here some version 10.x) separately, as packages with multiple version lines like Tomcat would benefit from this.

  • Not sure what you have in mind -- a discussion and/or examples would help flesh this out.

In general I feel avoiding the visual repetition of the purl everywhere when only the version changed would simplify the display.

  • Agreed -- it's a noticeable improvement.

Also what happens when:

  • there is no other vulnerabilities?
  • there is no package fixing a vulnerability?
  • there is no latest vulnerable?
  • If I understand your questions, these screen shots should answer them. The permutations are moderately involved and I might have missed some of them. Please let me know if that's not the case or if you have more questions/suggestions (heartily encouraged).

image

image

image

@pombredanne
Copy link
Member

pombredanne commented Aug 15, 2023

@johnmhoran Thanks!

Also we could consider the latest minor version (here some version 9.x) vs. the latest major version( here some version 10.x) separately, as packages with multiple version lines like Tomcat would benefit from this.

Not sure what you have in mind -- a discussion and/or examples would help flesh this out.

  • Say I have a vulnerability that affects Tomcat 9.3.0 and 10.4.1 that is fixed by 9.4.2 and 10.5.2.
  • Say that further we have the latest versions in each version line to be 9.6.7 and 10.8.2 that are also non vulnerable

Here for 9.3.0:

  • the closest non-vulnerable is 9.4.2
  • the latest non-vulnerable for 9.6.7

And for 10.4.1:

  • the closest non-vulnerable is 10.5.2
  • the latest non-vulnerable for 10.8.2

The idea is to treat the "major" version ranges (e.g., 9 and 10) almost as if they were different packages.

johnmhoran added a commit that referenced this issue Nov 22, 2023
… tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023


Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
…ated tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
…ng tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
…1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 22, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 23, 2023
…#1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
johnmhoran added a commit that referenced this issue Nov 29, 2023
Reference: #1228

Signed-off-by: John M. Horan johnmhoran@gmail.com
johnmhoran added a commit that referenced this issue Nov 29, 2023
Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
TG1999 added a commit that referenced this issue Nov 30, 2023
* Add initial fixed-affected-matching work #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Add Prefetch and univers-based version comparison #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Update affected-fixed package matching #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Improve matching and reporting code and UI #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Add univers version, revise sort and related code, update and add new tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Move weakness test #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Modify UI, update dictionary and tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Begin replacing strings with objects in package details dictionary #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Clean current package details template and related model code #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Begin work on major-version issue #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Complete first round of major-version vetting #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Remove major-version code, clean comments etc. #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Begin test refactoring #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Finish package details code and template, refactor/create package-related tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Refactor package details-related code #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Update Package details UI and Package API #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Fix 1 of 4 failing API tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Add initial fixed-affected-matching work #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Explore context and Package class approaches for affected-fixed package matching #1228

Reference: #1228

Note that my updated code is still in testing/dev stage and has not yet been completed or cleaned.

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Add Prefetch and univers-based version comparison #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Update affected-fixed package matching #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Improve matching and reporting code and UI #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Add univers version, revise sort and related code, update and add new tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Begin work on major-version issue #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Complete first round of major-version vetting #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Remove major-version code, clean comments etc. #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Begin test refactoring #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Finish package details code and template, refactor/create package-related tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Commit the initial refactoring changes from last week #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Refactor package details-related code #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Update Package details UI and Package API #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Save test experiments including commented-out variations #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Fix 1 of 4 failing API tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Update API including "lesser" fixed by versions, fix and update failing tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Update APITestCasePackage() class #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Test lack of "vulnerability" property #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Update get_affected_vulnerabilities() and test #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Update MinimalPackageSerializer() and missing-vulnerability-key test #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Append inside the if condition #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Update get_vulnerability() method #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Enable test_models.py and fix failing tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Update per PR comments #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Convert Package method to PackageQuerySet method, clean code and tests #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

* Fix failing tests

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

* Add property on functions in models

Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>

* Add and fix tests, address other comments #1228

Reference: #1228

Signed-off-by: John M. Horan <johnmhoran@gmail.com>

---------

Signed-off-by: John M. Horan <johnmhoran@gmail.com>
Signed-off-by: John M. Horan johnmhoran@gmail.com
Signed-off-by: Tushar Goel <tushar.goel.dav@gmail.com>
Co-authored-by: Tushar Goel <tushar.goel.dav@gmail.com>
@TG1999
Copy link
Contributor

TG1999 commented Nov 30, 2023

@johnmhoran thanks! done in #1249

@TG1999 TG1999 closed this as completed Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants