Add CVEChecker which guesses the pkg name and version of an archive #8

ahayzen · 2018-05-09T23:58:20Z

This is an initial implementation of a CVEChecker, for now it only guesses the pkg_name and version - which it then adds to the debug output.

Let me know if you want this behind a flag for now ? eg --experimental-cve-checker or if it is fine as is.

$ ./src/flatpak-external-data-checker --json --verbose ~/Downloads/com.skype.Client.json 
DEBUG:root:CVEChecker: Found libsecret of the version 0.18.5
DEBUG:root:libsecret-0.18.5.tar.xz is not a debian-repo type ext data
DEBUG:root:libsecret-0.18.5.tar.xz is not a rotating-url type ext data
DEBUG:root:CVEChecker: Found v4l-utils of the version 1.12.5
DEBUG:root:v4l-utils-1.12.5.tar.bz2 is not a debian-repo type ext data
DEBUG:root:v4l-utils-1.12.5.tar.bz2 is not a rotating-url type ext data
DEBUG:root:CVEChecker: Found nss of the version 3.36.1
DEBUG:root:nss-3.36.1.tar.gz is not a debian-repo type ext data
DEBUG:root:nss-3.36.1.tar.gz is not a rotating-url type ext data
DEBUG:root:CVEChecker: Unknown type Type.EXTRA_DATA
DEBUG:root:skypeforlinux-64.deb is not a debian-repo type ext data
DEBUG:root:skypeforlinux-64.deb is not a rotating-url type ext data

joaquimrocha · 2018-06-05T09:03:46Z

Hey @ahayzen ! I realize now I completely dropped the ball on this one! I will dedicate some time to review to it later today, sorry for such a delay. I will make sure I move faster now. Thanks for your patience and stay tuned.

joaquimrocha

Thanks a lot for this PR @ahayzen and I apologize again for the time it took me to get to it.
Please see my comments, the most important one is regarding the new field pkg_name which I think should really be part of the custom checker data.

joaquimrocha · 2018-06-05T23:21:34Z

src/checker.py

@@ -89,8 +94,8 @@ def _get_module_data_from_json(self, json_data):
                size = source.get('size', -1)
                checker_data = source.get('x-checker-data')

-                ext_data = ExternalData(data_type, name, url, sha256sum, size,
-                                        arches, checker_data)
+                ext_data = ExternalData(data_type, pkg_name, name, url,


From my previous comment, the pkg_name would be part of the checker_data here.

joaquimrocha · 2018-06-05T23:21:34Z

src/checker.py

@@ -68,6 +69,10 @@ def _get_finish_args_extra_data_from_json(self, json_data):
    def _get_module_data_from_json(self, json_data):
        external_data = []
        for module in json_data.get('modules', []):
+            # This is a guess at the package name from the name the author
+            # has given to the module block
+            pkg_name = module.get('name', None)


I think it makes more sense to have the package name as part of the checker data, this way we can enforce it for the CVE Checker.

joaquimrocha · 2018-06-05T23:22:16Z

src/checkers/cvechecker.py

+        try:
+            version = CVEChecker.extract_version_from_url(
+                external_data.url, external_data.type,
+            )


I really don't like the arguments split like this + the single parenthesis.

As we are using 100 chars for each line, moved this onto one line.

joaquimrocha · 2018-06-05T23:24:09Z

src/checkers/cvechecker.py

+            version = CVEChecker.extract_version_from_url(
+                external_data.url, external_data.type,
+            )
+            logging.debug('CVEChecker: Found %s of the version %s' %


I think this is the old string format, it should be "... Found {} of the versions {}...".format(...).

joaquimrocha · 2018-06-05T23:25:23Z

src/checkers/cvechecker.py

+                logging.debug('CVEChecker: Version not found in {}'.format(url))
+                raise ValueError
+        else:
+            logging.debug('CVEChecker: Unknown type %s' % data_type)


String format.

joaquimrocha · 2018-06-05T23:25:48Z

src/lib/externaldata.py

-                 checker_data=None):
+    def __init__(self, data_type, pkg_name, filename, url, checksum, size=-1,
+                 arches=[], checker_data=None):
+        self.pkg_name = pkg_name


See other comments regarding the pkg_name.

joaquimrocha · 2018-06-05T23:26:03Z

src/lib/externaldata.py

@@ -54,13 +56,15 @@ def __init__(self, data_type, filename, url, checksum, size=-1, arches=[],

    def __str__(self):
        info = '{filename}:\n' \
+               '  PkgName: {pkg_name}\n' \


Ditto for the pkg_name.

joaquimrocha · 2018-06-05T23:27:58Z

One extra comment: it'd be great if you could add a test for this, similar to what we have for the other checkers.

ahayzen · 2018-07-24T01:11:23Z

@joaquimrocha I have done the changes, now CVEChecker uses package-name from checker_data. I would like the package name to be auto determined in the future from the URL for archive type (optionally without x-checker-data) so that we can run across flathub easily.

Sure I'll write some tests next :-)

ahayzen · 2018-08-01T00:45:37Z

@joaquimrocha Hey, I've added a basic test case to this now (as we add more functionality to the CVEChecker itself the tests can be expanded). Please re-review when you have a moment :-)

joaquimrocha

Please see my comments in the commits.
Also, I thought this was somehow about checking https://cve.mitre.org/ but you're just printing the versions, right? Are you thinking of checking CVEs later or did I completely misunderstood the purpose of this checker?

joaquimrocha · 2018-08-03T16:19:54Z

src/checkers/cvechecker.py

+#
+# Authors:
+#       Andrew Hayzen <[email protected]>
+#       Joaquim Rocha <[email protected]>


(I don't mind but) you don't have to add my name if I didn't touch this code or if most of it wasn't copied from my code.

joaquimrocha · 2018-08-03T16:20:29Z

src/checkers/cvechecker.py

+
+    def check(self, external_data):
+        # TODO: if checker_data or package-name are None
+        # attempt to guess package name from url if archive


typo: from url IN archive?

joaquimrocha · 2018-08-03T16:22:59Z

src/lib/externaldata.py

@@ -49,6 +49,7 @@ def __init__(self, data_type, filename, url, checksum, size=-1, arches=[],
        self.arches = arches
        self.type = data_type
        self.checker_data = checker_data
+        self.current_version = None


This should be in checker_data.

joaquimrocha · 2018-08-03T16:24:46Z

src/checkers/cvechecker.py

@@ -0,0 +1,74 @@
+# Copyright (C) 2018 Endless Mobile, Inc.


You don't have to assign the copyright to Endless if you don't want and you're not using most of its code.
Also, please describe what this checker is about (see other checkers for examples).

joaquimrocha · 2018-08-03T16:27:32Z

tests/test_checker.py

@@ -30,6 +30,7 @@
 import checker

 TEST_MANIFEST = os.path.join(tests_dir, 'org.externaldatachecker.Manifest.json')
+CVE_TEST_MANIFEST = os.path.join(tests_dir, 'org.cvechecker.Manifest.json')


Hmm, this way other checkers don't run your manifest. Maybe it'd be better to add your tests to the big manifest?

ahayzen · 2018-08-18T21:16:18Z

@joaquimrocha yeah this pull was just a proof of concept of extracting the version numbers etc using TingPing's original script and integrating into your tool :-) Not submitting them to an external service (yet).

It looks like TingPing has something already working with bst and yocto recipes (and maybe flatpak manifests?) for the runtime's, we should check with him before going any further as it sounds much more developed and would be duplicated work.

nanonyme · 2020-01-06T15:58:13Z

Possibly worth it to also use https://gitlab.com/BuildStream/bst-external/blob/master/bst_external/elements/collect_manifest.py for inspiration. It's being used to generate CVE reports for freedesktop-sdk and Gnome runtimes.

ahayzen mentioned this pull request May 9, 2018

Initial experimental CVE checker (port of TingPing's script) #6

Closed

ahayzen force-pushed the cve-checker-read-version branch from abb9802 to c0031e2 Compare May 10, 2018 00:14

Add CVEChecker which guesses the pkg name and version of an archive

3651151

ahayzen force-pushed the cve-checker-read-version branch from c0031e2 to 3651151 Compare May 10, 2018 00:21

joaquimrocha suggested changes Jun 5, 2018

View reviewed changes

Use package-name from checker_data and fix formatting

0ba35a1

ahayzen added 2 commits July 24, 2018 02:16

Reduce diff

8ca2f8a

Add basic test case for CVEChecker

422c9ba

joaquimrocha suggested changes Aug 3, 2018

View reviewed changes

vchernin mentioned this pull request Feb 10, 2022

Support only opening PRs for certain sources #249

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CVEChecker which guesses the pkg name and version of an archive #8

Add CVEChecker which guesses the pkg name and version of an archive #8

ahayzen commented May 9, 2018

joaquimrocha commented Jun 5, 2018

joaquimrocha left a comment

joaquimrocha Jun 5, 2018

ahayzen Jul 24, 2018

joaquimrocha Jun 5, 2018

ahayzen Jul 24, 2018

joaquimrocha Jun 5, 2018

ahayzen Jul 24, 2018

joaquimrocha Jun 5, 2018

ahayzen Jul 24, 2018

joaquimrocha Jun 5, 2018

ahayzen Jul 24, 2018

joaquimrocha Jun 5, 2018

ahayzen Jul 24, 2018

joaquimrocha Jun 5, 2018

ahayzen Jul 24, 2018

joaquimrocha commented Jun 5, 2018

ahayzen commented Jul 24, 2018

ahayzen commented Aug 1, 2018

joaquimrocha left a comment

joaquimrocha Aug 3, 2018

joaquimrocha Aug 3, 2018

joaquimrocha Aug 3, 2018

joaquimrocha Aug 3, 2018

joaquimrocha Aug 3, 2018

ahayzen commented Aug 18, 2018

nanonyme commented Jan 6, 2020

Add CVEChecker which guesses the pkg name and version of an archive #8

Are you sure you want to change the base?

Add CVEChecker which guesses the pkg name and version of an archive #8

Conversation

ahayzen commented May 9, 2018

joaquimrocha commented Jun 5, 2018

joaquimrocha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joaquimrocha commented Jun 5, 2018

ahayzen commented Jul 24, 2018

ahayzen commented Aug 1, 2018

joaquimrocha left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahayzen commented Aug 18, 2018

nanonyme commented Jan 6, 2020