Updated regex_find in utils.py and all the checkers using it #331
Conversation
uptodate with intel cve-bin-tool
|
@terriko The checks are showing the following error. Tests were fine on my PC. cve-bin-tool / Run tests (ubuntu-latest, 3.6) (pull_request) cve-bin-tool / Run tests (ubuntu-latest, 3.7) (pull_request) cve-bin-tool / Long tests on python3.8 (pull_request) |
|
@SinghHrmn Yes the |
|
@PrajwalM2212 Okay -- thanks for the reply. |
There was a problem hiding this comment.
I like the idea of this patch, but I think we'd be better served if regex_find returned "UNKNOWN" and then we do the correct thing with that in cli.py. Why? The giant "UNKNOWN" shows up nicely in all the debug logs and makes it easier for users to tell that the checker has failed to find a version . I know, I know, it makes it a string compare instead of a binary one, but we've had so many people confused about checker behaviours that I feel like it's worth it.
|
@terriko I agree on the point that it's making checkers look confusing. But you can see from the code that some checkers are using regex_find and expect it would return False or anything similar that makes the if statement logical. Like this code from kerberos.py will work even if the regex_find is returning an empty string "". Due to the following code reason. x = ""
if x:
print("it is working")
else:
print("not working")
#prints not workingBut according to your proposed changes. The changes will look something like. to But after this, I have to update the if logic to After this, what should we use?
|
|
@terriko Submitting a PR with regex_find returning "UNKNOWN". Long tests as well simple tests were fine. |
|
I guess I wasn't clear in what I was suggesting. Yes, cli.py doesn't do the right thing yet. I was trying to suggest that this patch should fix cli.py as well. You pretty much had the right idea in #326 already, I think. Basically, what I was trying to suggest is probably 3 things:
I think that'll get us improved output and maybe a little performance improvement for not looking up meaningless data. Great work on this, by the way! This is a really great place for an improvement. |
|
@terriko Updated cli.py now the output will be like (venvgsoc) mastervulcan@DESKTOP-DMEM457:~/gsoc-cve-bin-tool/cve-bin-tool$ python -m cve_bin_tool.cli -x ../testFilesCVE-bin-tool/bluez_5.48-0ubuntu3.2_amd64.deb
cve_bin_tool.CVEDB - INFO - Using cached CVE data (<24h old). Use -u now to update immediately.
cve_bin_tool.Scanner - INFO - Checkers: bluez, curl, expat, ffmpeg, gnutls, icu, kerberos, libcurl, libgcrypt, libjpeg, libnss, libtiff, node, openssh, openssl, png, sqlite, systemd, xerces, xml2, zlib
cve_bin_tool - INFO - None
cve_bin_tool.Scanner - ERROR - No version info for 'bluetoothctl'
cve_bin_tool.Scanner - WARNING - 'bluetoothctl' was detected with version UNKNOWN
cve_bin_tool.Scanner - ERROR - No version info for 'bluetoothctl'
cve_bin_tool.Scanner - WARNING - 'bluetoothctl' was detected with version UNKNOWN
cve_bin_tool - INFO -
cve_bin_tool - INFO - Overall CVE summary:
cve_bin_tool - INFO - There are 0 files with known CVEs detected
(venvgsoc) mastervulcan@DESKTOP-DMEM457:~/gsoc-cve-bin-tool/cve-bin-tool$ python -m cve_bin_tool.cli -x ../testFilesCVE-bin-tool/gnutls-bin_3.5.8-6ubuntu3_amd64.deb
cve_bin_tool.CVEDB - INFO - Using cached CVE data (<24h old). Use -u now to update immediately.
cve_bin_tool.Scanner - INFO - Checkers: bluez, curl, expat, ffmpeg, gnutls, icu, kerberos, libcurl, libgcrypt, libjpeg, libnss, libtiff, node, openssh, openssl, png, sqlite, systemd, xerces, xml2, zlib
cve_bin_tool - INFO - None
cve_bin_tool.Scanner - INFO - /tmp/cve-bin-tool-yv_yiqu2/data.tar.xz.extracted/usr/bin/gnutls-cli is gnutls-cli 3.5.8
cve_bin_tool.Scanner - INFO - Known CVEs in version 3.5.8
cve_bin_tool.Scanner - INFO - CVE-2009-1390, CVE-2014-3467, CVE-2014-3468, CVE-2014-3469, CVE-2017-7507, CVE-2017-7869, CVE-2019-3829
cve_bin_tool.Scanner - ERROR - No version info for 'gnutls-cli'
cve_bin_tool.Scanner - WARNING - 'gnutls-cli' was detected with version UNKNOWN
cve_bin_tool.Scanner - ERROR - No version info for 'gnutls-serv'
cve_bin_tool.Scanner - WARNING - 'gnutls-serv' was detected with version UNKNOWN
cve_bin_tool.Scanner - INFO - /tmp/cve-bin-tool-yv_yiqu2/gnutls-bin_3.5.8-6ubuntu3_amd64.deb.extracted/usr/bin/gnutls-cli is gnutls-cli 3.5.8
cve_bin_tool.Scanner - INFO - Known CVEs in version 3.5.8
cve_bin_tool.Scanner - INFO - CVE-2009-1390, CVE-2014-3467, CVE-2014-3468, CVE-2014-3469, CVE-2017-7507, CVE-2017-7869, CVE-2019-3829
cve_bin_tool.Scanner - ERROR - No version info for 'gnutls-cli'
cve_bin_tool.Scanner - WARNING - 'gnutls-cli' was detected with version UNKNOWN
cve_bin_tool.Scanner - ERROR - No version info for 'gnutls-serv'
cve_bin_tool.Scanner - WARNING - 'gnutls-serv' was detected with version UNKNOWN
cve_bin_tool - INFO -
cve_bin_tool - INFO - Overall CVE summary:
cve_bin_tool - INFO - There are 2 files with known CVEs detected
cve_bin_tool - INFO - Known CVEs in ('gnutls-cli', '3.5.8'):
gnutls-cli,3.5.8,CVE-2009-1390,MEDIUM
gnutls-cli,3.5.8,CVE-2014-3467,MEDIUM
gnutls-cli,3.5.8,CVE-2014-3468,MEDIUM
gnutls-cli,3.5.8,CVE-2014-3469,MEDIUM
gnutls-cli,3.5.8,CVE-2017-7507,HIGH
gnutls-cli,3.5.8,CVE-2017-7869,HIGH
gnutls-cli,3.5.8,CVE-2019-3829,HIGHyou can suggest me if to only show warning or error (or both). But one thing is for sure if the error happens it will satisfy both and will print both. |
|
@terriko closing this one and trying a new pull request |
|
@terriko tests were fine at my system. But I don't know why I got this error I didn't even touch that file. more over I didn't find that test either ======================================================================
FAIL: test_icu_dos (test.test_scanner.TestScanner)
Test a pathologically long version string for icu
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/runner/work/cve-bin-tool/cve-bin-tool/test/test_scanner.py", line 161, in test_icu_dos
["CVE-2019-3823"],
File "/home/runner/work/cve-bin-tool/cve-bin-tool/test/test_scanner.py", line 95, in _binary_test
self.assertIn(version, list(cves[package].keys()))
AssertionError: '3.8.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1.1' not found in ['3.8.1']
----------------------------------------------------------------------
Ran 91 tests in 205.641s
FAILED (failures=1, skipped=26)
Test failed: <unittest.runner.TextTestResult run=91 errors=0 failures=1>
error: Test failed: <unittest.runner.TextTestResult run=91 errors=0 failures=1>
|
|
That is weird. That's one of the new tests I just put in on the more open-ended regular expressions, but it looks like it's getting a truncated, more normal-looking version number instead of the ridiculous fake one I sent it. That would make sense if you'd still had changes in icu, but you don't so I'm confused. As for error vs warning: I think warning is better, since we know there's a few cases where we're just not finding data and that's just going to happen. The error was something I put in there pretty recently so I doubt anyone's relying on it; it should be safe to remove it in favour of a if version/elif version==unknown/else construction. |
|
Double-checked on my local machine by merging this branch against master, and I'm seeing this test fail now too, so it's definitely not a quirk of the CI system. Something here has changed it so we're getting a truncated version result. |
@terriko Please help and look into this error. Is there any way how we can remove this error. It's making my PR fail every time. |
|
@terriko Found the error. It was same as kerberos.py. icu.py was not returning UNKNOWN which was making cli.py inconsistent. You can see that in file changes. |
|
@terriko Finally I'm feeling happy. I hope this PR will be beneficial for our tool. :) |
Currently, the regex_find function in the utils.py initializes a string variable guess = "". And returns this empty string if it is unable to find the match. This change will help omit those empty string versions which were detected falsely by returning a False statement if guess == "".
Output will change from
To
Even if the checker failed to check for a version. The default value of version = "UNKNOWN" will be used. Hence giving the below output