Skip to content

Added csv output option #354

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

Closed
wants to merge 3 commits into from
Closed

Conversation

k-udupa2000
Copy link
Contributor

Created command line flag -c
Command used for testing

python -m cve_bin_tool.cli -x ./ppp-2.4.7-26.fc29.x86_64.rpm -c

Sample output:
,openssl,openssl,CVE-1999-0428,HIGH
,openssl,openssl,CVE-2000-1254,HIGH
,openssl,openssl,CVE-2006-4339,MEDIUM
,openssl,openssl,CVE-2006-7250,MEDIUM....

@k-udupa2000 k-udupa2000 requested a review from terriko February 14, 2020 02:33
@terriko
Copy link
Contributor

terriko commented Feb 14, 2020

I think something got screwed up here -- this has an exif test in it that will fail because there's no exif checker. You may need to rebase off master to get rid of it.

@@ -200,6 +202,8 @@ def scan_file(self, filename):
)

found_cves = self.get_cves(vendor_package_pairs, version)
vendor = vendor_package_pairs[0][0]
package = vendor_package_pairs[0][1]
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens here if there's more than one vendor package pair? (maybe try it against a kerberos test case to find out if you don't know)

@@ -353,7 +376,9 @@ def main(argv=None, outfile=sys.stdout):
)

parser.add_argument("-q", "--quiet", action="store_true", help="suppress output")

parser.add_argument(
"-c", "--csv", action="store_true", help="Stores the output in csv file."
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm fairly sure we're going to want more than one type of output, so let's make this a --output flag or maybe an --output-type flag (so you can use --output for a filename). Or maybe make it figure it out from the extension on a filename, defaulting to the usual console output but giving you .csv if your file ends in .csv. (And eventually, .html, .json, others)

@@ -406,6 +431,9 @@ def main(argv=None, outfile=sys.stdout):
# update db if needed
if args.update != "never":
cvedb_orig.get_cvelist_if_stale()
if args.csv:
global output_csv_file
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably better to let people specify their own filename, and print to console if they don't.

@@ -0,0 +1,71 @@
,openssl,openssl,CVE-1999-0428,HIGH
Copy link
Contributor

Choose a reason for hiding this comment

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

This file was probably checked in by accident and should be removed.

printf("They appear below this line.");
printf("------------------");
printf("libexif-12.mo");
printf("The EXIF library allows you to parse an EXIF file and read the data from those tags.");
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like a test that didn't work out and should also be removed since it's irrelevant to this PR.

@@ -32,6 +32,8 @@
from .cvedb import CVEDB
from .log import LOGGER

output_csv_file = 0
fileOpenMode = "w"
Copy link
Contributor

Choose a reason for hiding this comment

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

You have to open the file in write mode every time, so this doesn't need to be a variable, let alone a global one.

@@ -208,8 +212,27 @@ def scan_file(self, filename):
% (filename, result["is_or_contains"], modulename, version)
)
if found_cves.keys():
self.logger.info("Known CVEs in version " + str(version))
self.logger.info(", ".join(found_cves.keys()))
if output_csv_file:
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be a good time to refactor the output code (that's why this was listed as part of a gsoc project and not something we'd planned to just do a quick fix on) so that you can have a function that goes something like cve_output("csv", cves) or cve_output("console", cves) and not have the mess of output that we have now.

@k-udupa2000
Copy link
Contributor Author

  1. @terriko Thanks for the detailed explanation!
    I have couple of doubts regarding the code:
    a. This code is failing when I run it for test present in test_scanner. This is because the main function in cli.py is not executed and hence the arguments are not declared. Could you guide me through this.
    b. Write mode in the file was erasing the previous contents of the file, hence I used a variable for it.

Lastly, Since this is listed under GSOC project Idea, should this be left for GSOC?

@terriko
Copy link
Contributor

terriko commented Feb 18, 2020

  • Yes, our tests for cli.py test the sub-functions, and you'd need to have the tests work differently here than they do for the scanner portions, since you're testing a different thing. If you're breaking the existing tests, though, that's a bad sign.
  • Using a variable vs not shouldn't make any difference in the behaviour of write, and I think you actually want to overwrite the file in this case anyhow (because how would you tell when you fixed CVEs in subsequent scans otherwise?)

You don't have to leave this for GSoC, but it's a change that's going to require more than a few days of work and refactoring to be really excellent, so that's why we put it as a GSoC option. It's a big enough change that I'd like the contributor who does it to get paid for the work if we can.

There's a whole debate about the ethics of open source contributions and getting paid, but I won't turn down unpaid labour if you just want to do it. Unpaid contributions is how I got my start and continue to learn new technologies, so I feel like I'd be betraying my teenaged self if I stopped people from trying to do big things. But if you're eligible for GSoC it's totally okay to look at this say "Look, I got this far" and put it in your application to get paid to finish the work. :)

@terriko
Copy link
Contributor

terriko commented Feb 29, 2020

Superceeded by #410

@terriko terriko closed this Feb 29, 2020
@k-udupa2000 k-udupa2000 deleted the addcsv branch November 10, 2020 19:04
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