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

Handle removed CPEs and CVEs in SCAP sync #1097

Merged
merged 7 commits into from
May 20, 2020

Conversation

mattmundell
Copy link
Contributor

@mattmundell mattmundell commented May 18, 2020

Checklist:

@mattmundell mattmundell changed the title Remove no longer affected products when updating CVE Handle removed CPEs and CVEs in SCAP sync May 19, 2020
@mattmundell mattmundell marked this pull request as ready for review May 19, 2020 12:48
Copy link
Member

@timopollmeier timopollmeier left a comment

Choose a reason for hiding this comment

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

This looks okay overall but unless I'm missing something this doesn't remove affected products that were removed from CVEs that are still in the XML.

I also wonder if using a temporary table or other data structure instead of buffering an array literal wouldn't be a better idea, especially when the CPEs from CVEs could have some redundancy.

@mattmundell
Copy link
Contributor Author

This looks okay overall but unless I'm missing something this doesn't remove affected products that were removed from CVEs that are still in the XML.

This is done by the changes to insert_cve_products in 2879642.

It collects all affected products for the CVE in initial_affected, then removes the CPEs from init_affected as they are encountered. At the end it removes any that remain.

Or do you mean something else?

@mattmundell
Copy link
Contributor Author

I also wonder if using a temporary table or other data structure instead of buffering an array literal wouldn't be a better idea, especially when the CPEs from CVEs could have some redundancy.

I wanted to avoid the SQL queries that would be required for a temporary table. Apparently the ANY(array...) will be converted to a temporary table by Postgres anyway.

I don't think duplicates will be a problem. The list should be small. (Admittedly a small list mean using SQL queries to fill a temporary table may not be a problem.)

@mattmundell mattmundell requested a review from timopollmeier May 20, 2020 09:39
Copy link
Member

@timopollmeier timopollmeier left a comment

Choose a reason for hiding this comment

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

Sorry, looks like I got CVEs and CPEs confused in some places when I first read this PR.

Having all the CVE-IDs in a single literal still seems kind of long, but as long as there are also no problems with reallocating the memory for the string that often, it should be fine.

@mattmundell mattmundell merged commit 2f15400 into greenbone:master May 20, 2020
@mattmundell mattmundell deleted the cpe-removal branch May 20, 2020 13:12
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.

2 participants