Skip to content

Fix some bug risks and quality issues #243

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 11, 2019
Merged

Fix some bug risks and quality issues #243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions cve_bin_tool/NVDAutoUpdate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,9 @@ def find_curl_list(
):
""" Extract curl data """
# import urllib.request
import re

cve_pattern = re.compile('name=(CVE-[^"]*)')
nextver_pattern = re.compile("the subsequent release: ([\d.]+)")
nextver_pattern = re.compile(r"the subsequent release: ([\d.]+)")

# Start with version 6.0 since that's currently first
version = "6.0"
Expand Down
2 changes: 1 addition & 1 deletion cve_bin_tool/checkers/expat.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/python3
# pylint: disable=anomalous-backslash-in-string, invalid-name
"""
r"""
Copy link

Choose a reason for hiding this comment

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

I don't think we want a r prefix on this docstring

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's an unescaped backslash in the docstring on line 16. Adding another backslash to escape it can change the meaning, which is why we can convert the docstring to a raw string.

CVE checker for libexpat
References:
Expand Down
6 changes: 2 additions & 4 deletions cve_bin_tool/checkers/openssh.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
References:
https://www.cvedetails.com/product/585/Openbsd-Openssh.html?vendor_id=97
"""
from ..util import regex_find

import sys, re
import re


def get_version(lines, filename):
Expand All @@ -17,7 +15,7 @@ def get_version(lines, filename):

VPkg: openbsd, openssh
"""
regex = re.compile("OpenSSH_([0-9]+\.[0-9]+[0-9a-z\s]*)")
regex = re.compile(r"OpenSSH_([0-9]+\.[0-9]+[0-9a-z\s]*)")
version_info = dict()

# determine version
Expand Down
8 changes: 5 additions & 3 deletions cve_bin_tool/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import sys
import os
import csv
import glob
import platform
import subprocess
import logging
Expand Down Expand Up @@ -120,7 +119,7 @@ def get_cves(self, vendor_package_pairs, vers):
"""
cves = defaultdict(list)

for i in range(len(vendor_package_pairs)):
for i, _ in enumerate(vendor_package_pairs):
vendor_package_pairs[i] = tuple(vendor_package_pairs[i])[:2] + (
"%" + str(vers) + "%",
)
Expand Down Expand Up @@ -290,8 +289,11 @@ def output_cves(outfile, modules, include_details=False):
writer.writerow(row)


def main(argv=sys.argv, outfile=sys.stdout):
def main(argv=None, outfile=sys.stdout):
""" Scan a binary file for certain open source libraries that may have CVEs """
if argv is None:
argv = sys.argv
Copy link

Choose a reason for hiding this comment

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

What's the purpose of this change?

Copy link

Choose a reason for hiding this comment

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

I see your comment says this makes it not mutable, what do you mean by this? My understanding is that this changes preserves mutability. If we wished to make changes to argv not change sys.argv, we'd have to use copy.deepcopy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is related to sys.argv being passed as a default argument in the function definition (on line 292).

def main(argv=sys.argv, outfile=sys.stdout):
    ...

It's not recommended to pass mutable objects as default values since the latest passed value is preserved on subsequent function calls, and can cause unintended effects. (ref)

Copy link

Choose a reason for hiding this comment

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

Ah okay I see what you mean, cool!


parser = argparse.ArgumentParser(
prog="cve-bin-tool",
description="The CVE Binary Tool scans for a number of common, vulnerable open source components (openssl, libpng, libxml2, expat and a few others) to let you know if a given directory or binary file includes common libraries with known vulnerabilities.",
Expand Down
8 changes: 7 additions & 1 deletion cve_bin_tool/csv2cve.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,16 @@
ERR_MISSINGCOLUMN = -2


def main(argv=sys.argv, outfile=sys.stdout):
def main(argv=None, outfile=None):
""" Take a list of package information + versions from a CSV file,
and output a list of matching CVES """

if argv is None:
argv = sys.argv

if outfile is None:
outfile = sys.stdout

parser = argparse.ArgumentParser(
prog="csv2cve",
description="This tool takes a list of software + versions from a CSV file and outputs a list of CVEs known to affect those versions",
Expand Down