From 2bc84138c8e1bfec82f8cf33aeb99f472f5cdfa2 Mon Sep 17 00:00:00 2001 From: bcaller Date: Wed, 5 Sep 2018 14:47:46 +0100 Subject: [PATCH 1/4] Add colourful formatter "screen" Prints vulnerabilities with ANSI colour codes for the terminal. Not crazily colourful: just tries to highlight the important stuff. Repeated filenames aren't printed. Colour scheme might not be to everyone's taste. --- .coveragerc | 1 + pyt/formatters/screen.py | 104 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 pyt/formatters/screen.py diff --git a/.coveragerc b/.coveragerc index df3137cf..26a84f47 100644 --- a/.coveragerc +++ b/.coveragerc @@ -15,4 +15,5 @@ source = ./tests omit = pyt/formatters/json.py + pyt/formatters/screen.py pyt/formatters/text.py diff --git a/pyt/formatters/screen.py b/pyt/formatters/screen.py new file mode 100644 index 00000000..3a8ddb80 --- /dev/null +++ b/pyt/formatters/screen.py @@ -0,0 +1,104 @@ +"""This formatter outputs the issues as color-coded text.""" +from ..vulnerabilities.vulnerability_helper import SanitisedVulnerability, UnknownVulnerability + +RESET = '033円[0m' +BOLD = '033円[1m' +UNDERLINE = '033円[4m' +DANGER = '033円[31m' +GOOD = '033円[32m' +HIGHLIGHT = '033円[45;1m' +RED_ON_WHITE = '033円[31m033円[107m' + + +def color(string, color_string): + return color_string + str(string) + RESET + + +def report( + vulnerabilities, + fileobj, + print_sanitised, +): + """ + Prints issues in color-coded text format. + + Args: + vulnerabilities: list of vulnerabilities to report + fileobj: The output file object, which may be sys.stdout + """ + n_vulnerabilities = len(vulnerabilities) + unsanitised_vulnerabilities = [v for v in vulnerabilities if not isinstance(v, SanitisedVulnerability)] + n_unsanitised = len(unsanitised_vulnerabilities) + n_sanitised = n_vulnerabilities - n_unsanitised + heading = "{} vulnerabilit{} found{}.\n".format( + 'No' if n_unsanitised == 0 else n_unsanitised, + 'y' if n_unsanitised == 1 else 'ies', + " (plus {} sanitised)".format(n_sanitised) if n_sanitised else "", + ) + vulnerabilities_to_print = vulnerabilities if print_sanitised else unsanitised_vulnerabilities + with fileobj: + for i, vulnerability in enumerate(vulnerabilities_to_print, start=1): + fileobj.write(vulnerability_to_str(i, vulnerability)) + + if n_unsanitised == 0: + fileobj.write(color(heading, GOOD)) + else: + fileobj.write(color(heading, DANGER)) + + +def vulnerability_to_str(i, vulnerability): + lines = [] + lines.append(color('Vulnerability {}'.format(i), UNDERLINE)) + lines.append('File: {}'.format(color(vulnerability.source.path, BOLD))) + lines.append( + 'User input at line {}, source "{}":'.format( + vulnerability.source.line_number, + color(vulnerability.source_trigger_word, HIGHLIGHT), + ) + ) + lines.append('\t{}'.format(color(vulnerability.source.label, RED_ON_WHITE))) + if vulnerability.reassignment_nodes: + previous_path = None + lines.append('Reassigned in:') + for node in vulnerability.reassignment_nodes: + if node.path != previous_path: + lines.append('\tFile: {}'.format(node.path)) + previous_path = node.path + label = node.label + if ( + isinstance(vulnerability, SanitisedVulnerability) and + node.label == vulnerability.sanitiser.label + ): + label = color(label, GOOD) + lines.append( + '\t Line {}:\t{}'.format( + node.line_number, + label, + ) + ) + if vulnerability.source.path != vulnerability.sink.path: + lines.append('File: {}'.format(color(vulnerability.sink.path, BOLD))) + lines.append( + 'Reaches line {}, sink "{}"'.format( + vulnerability.sink.line_number, + color(vulnerability.sink_trigger_word, HIGHLIGHT), + ) + ) + lines.append('\t{}'.format( + color(vulnerability.sink.label, RED_ON_WHITE) + )) + if isinstance(vulnerability, SanitisedVulnerability): + lines.append( + 'This vulnerability is {}{} by {}'.format( + color('potentially ', BOLD) if not vulnerability.confident else '', + color('sanitised', GOOD), + color(vulnerability.sanitiser.label, BOLD), + ) + ) + elif isinstance(vulnerability, UnknownVulnerability): + lines.append( + 'This vulnerability is unknown due to "{}"'.format( + color(vulnerability.unknown_assignment.label, BOLD), + ) + ) + return '\n'.join(lines) + '\n\n' From 4f80f97286cdfc5b32488a04eec7ecb49019297a Mon Sep 17 00:00:00 2001 From: bcaller Date: Wed, 5 Sep 2018 14:50:41 +0100 Subject: [PATCH 2/4] Consistent spelling of sanitise --- pyt/__main__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyt/__main__.py b/pyt/__main__.py index 558f1d3c..4bea43d2 100644 --- a/pyt/__main__.py +++ b/pyt/__main__.py @@ -135,11 +135,11 @@ def main(command_line_args=sys.argv[1:]): # noqa: C901 else: text.report(vulnerabilities, args.output_file) - has_unsanitized_vulnerabilities = any( + has_unsanitised_vulnerabilities = any( not isinstance(v, SanitisedVulnerability) for v in vulnerabilities ) - if has_unsanitized_vulnerabilities: + if has_unsanitised_vulnerabilities: sys.exit(1) From c07551dbcfcd369570ff9e1dfadafa18e8acc060 Mon Sep 17 00:00:00 2001 From: bcaller Date: Wed, 5 Sep 2018 14:48:56 +0100 Subject: [PATCH 3/4] Add --only-unsanitised flag to not print sanitised vulnerabilities It is sometimes what you want, but often you just want the failures without sanitised vulns in the output. --- pyt/__main__.py | 9 +-------- pyt/formatters/json.py | 11 ++++++++--- pyt/formatters/text.py | 26 +++++++++++++++++--------- pyt/usage.py | 30 ++++++++++++++++++++++++------ tests/main_test.py | 33 +++++++++++++++++++++------------ tests/usage_test.py | 15 +++++++++------ 6 files changed, 80 insertions(+), 44 deletions(-) diff --git a/pyt/__main__.py b/pyt/__main__.py index 4bea43d2..3e6a9911 100644 --- a/pyt/__main__.py +++ b/pyt/__main__.py @@ -12,10 +12,6 @@ get_directory_modules, get_modules ) -from .formatters import ( - json, - text -) from .usage import parse_args from .vulnerabilities import ( find_vulnerabilities, @@ -130,10 +126,7 @@ def main(command_line_args=sys.argv[1:]): # noqa: C901 args.baseline ) - if args.json: - json.report(vulnerabilities, args.output_file) - else: - text.report(vulnerabilities, args.output_file) + args.formatter.report(vulnerabilities, args.output_file, not args.only_unsanitised) has_unsanitised_vulnerabilities = any( not isinstance(v, SanitisedVulnerability) diff --git a/pyt/formatters/json.py b/pyt/formatters/json.py index efc95b95..8e0eab11 100644 --- a/pyt/formatters/json.py +++ b/pyt/formatters/json.py @@ -1,12 +1,14 @@ """This formatter outputs the issues in JSON.""" - import json from datetime import datetime +from ..vulnerabilities.vulnerability_helper import SanitisedVulnerability + def report( vulnerabilities, - fileobj + fileobj, + print_sanitised, ): """ Prints issues in JSON format. @@ -19,7 +21,10 @@ def report( machine_output = { 'generated_at': time_string, - 'vulnerabilities': [vuln.as_dict() for vuln in vulnerabilities] + 'vulnerabilities': [ + vuln.as_dict() for vuln in vulnerabilities + if print_sanitised or not isinstance(vuln, SanitisedVulnerability) + ] } result = json.dumps( diff --git a/pyt/formatters/text.py b/pyt/formatters/text.py index 7961b05e..2041e006 100644 --- a/pyt/formatters/text.py +++ b/pyt/formatters/text.py @@ -1,9 +1,11 @@ """This formatter outputs the issues as plain text.""" +from ..vulnerabilities.vulnerability_helper import SanitisedVulnerability def report( vulnerabilities, - fileobj + fileobj, + print_sanitised, ): """ Prints issues in text format. @@ -11,15 +13,21 @@ def report( Args: vulnerabilities: list of vulnerabilities to report fileobj: The output file object, which may be sys.stdout + print_sanitised: Print just unsanitised vulnerabilities or sanitised vulnerabilities as well """ - number_of_vulnerabilities = len(vulnerabilities) + n_vulnerabilities = len(vulnerabilities) + unsanitised_vulnerabilities = [v for v in vulnerabilities if not isinstance(v, SanitisedVulnerability)] + n_unsanitised = len(unsanitised_vulnerabilities) + n_sanitised = n_vulnerabilities - n_unsanitised + heading = "{} vulnerabilit{} found{}{}\n".format( + 'No' if n_unsanitised == 0 else n_unsanitised, + 'y' if n_unsanitised == 1 else 'ies', + " (plus {} sanitised)".format(n_sanitised) if n_sanitised else "", + ':' if n_vulnerabilities else '.', + ) + vulnerabilities_to_print = vulnerabilities if print_sanitised else unsanitised_vulnerabilities with fileobj: - if number_of_vulnerabilities == 0: - fileobj.write('No vulnerabilities found.\n') - elif number_of_vulnerabilities == 1: - fileobj.write('%s vulnerability found:\n' % number_of_vulnerabilities) - else: - fileobj.write('%s vulnerabilities found:\n' % number_of_vulnerabilities) + fileobj.write(heading) - for i, vulnerability in enumerate(vulnerabilities, start=1): + for i, vulnerability in enumerate(vulnerabilities_to_print, start=1): fileobj.write('Vulnerability {}:\n{}\n\n'.format(i, vulnerability)) diff --git a/pyt/usage.py b/pyt/usage.py index d5b6efbe..7acc5be7 100644 --- a/pyt/usage.py +++ b/pyt/usage.py @@ -2,6 +2,8 @@ import os import sys +from .formatters import json, screen, text + default_blackbox_mapping_file = os.path.join( os.path.dirname(__file__), @@ -49,12 +51,6 @@ def _add_optional_group(parser): default=False, metavar='BASELINE_JSON_FILE', ) - optional_group.add_argument( - '-j', '--json', - help='Prints JSON instead of report.', - action='store_true', - default=False - ) optional_group.add_argument( '-t', '--trigger-word-file', help='Input file with a list of sources and sinks', @@ -115,6 +111,28 @@ def _add_optional_group(parser): default=True, dest='allow_local_imports' ) + optional_group.add_argument( + '-u', '--only-unsanitised', + help="Don't print sanitised vulnerabilities.", + action='store_true', + default=False, + ) + parser.set_defaults(formatter=text) + formatter_group = optional_group.add_mutually_exclusive_group() + formatter_group.add_argument( + '-j', '--json', + help='Prints JSON instead of report.', + action='store_const', + const=json, + dest='formatter', + ) + formatter_group.add_argument( + '-s', '--screen', + help='Prints colorful report.', + action='store_const', + const=screen, + dest='formatter', + ) def parse_args(args): diff --git a/tests/main_test.py b/tests/main_test.py index 1037d843..1e33ee24 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -8,72 +8,81 @@ class MainTest(BaseTestCase): @mock.patch('pyt.__main__.discover_files') @mock.patch('pyt.__main__.parse_args') @mock.patch('pyt.__main__.find_vulnerabilities') - @mock.patch('pyt.__main__.text') + @mock.patch('pyt.formatters.text') def test_text_output(self, mock_text, mock_find_vulnerabilities, mock_parse_args, mock_discover_files): mock_find_vulnerabilities.return_value = 'stuff' example_file = 'examples/vulnerable_code/inter_command_injection.py' output_file = 'mocked_outfile' + import pyt.formatters.text mock_discover_files.return_value = [example_file] mock_parse_args.return_value = mock.Mock( project_root=None, baseline=None, - json=None, - output_file=output_file + formatter=pyt.formatters.text, + output_file=output_file, + only_unsanitised=False, ) with self.assertRaises(SystemExit): main(['parse_args is mocked']) assert mock_text.report.call_count == 1 mock_text.report.assert_called_with( mock_find_vulnerabilities.return_value, - mock_parse_args.return_value.output_file + mock_parse_args.return_value.output_file, + True, ) @mock.patch('pyt.__main__.discover_files') @mock.patch('pyt.__main__.parse_args') @mock.patch('pyt.__main__.find_vulnerabilities') - @mock.patch('pyt.__main__.text') + @mock.patch('pyt.formatters.text') def test_no_vulns_found(self, mock_text, mock_find_vulnerabilities, mock_parse_args, mock_discover_files): mock_find_vulnerabilities.return_value = [] example_file = 'examples/vulnerable_code/inter_command_injection.py' output_file = 'mocked_outfile' + import pyt.formatters.text mock_discover_files.return_value = [example_file] mock_parse_args.return_value = mock.Mock( project_root=None, baseline=None, - json=None, - output_file=output_file + formatter=pyt.formatters.text, + output_file=output_file, + only_unsanitised=True, ) main(['parse_args is mocked']) # No SystemExit assert mock_text.report.call_count == 1 mock_text.report.assert_called_with( mock_find_vulnerabilities.return_value, - mock_parse_args.return_value.output_file + mock_parse_args.return_value.output_file, + False, ) @mock.patch('pyt.__main__.discover_files') @mock.patch('pyt.__main__.parse_args') @mock.patch('pyt.__main__.find_vulnerabilities') - @mock.patch('pyt.__main__.json') + @mock.patch('pyt.formatters.json') def test_json_output(self, mock_json, mock_find_vulnerabilities, mock_parse_args, mock_discover_files): mock_find_vulnerabilities.return_value = 'stuff' example_file = 'examples/vulnerable_code/inter_command_injection.py' output_file = 'mocked_outfile' + import pyt.formatters.json mock_discover_files.return_value = [example_file] mock_parse_args.return_value = mock.Mock( project_root=None, baseline=None, - json=True, - output_file=output_file + formatter=pyt.formatters.json, + output_file=output_file, + only_unsanitised=False, ) with self.assertRaises(SystemExit): main(['parse_args is mocked']) assert mock_json.report.call_count == 1 mock_json.report.assert_called_with( mock_find_vulnerabilities.return_value, - mock_parse_args.return_value.output_file + mock_parse_args.return_value.output_file, + True, ) diff --git a/tests/usage_test.py b/tests/usage_test.py index 363786bd..294004a4 100644 --- a/tests/usage_test.py +++ b/tests/usage_test.py @@ -26,10 +26,10 @@ def test_no_args(self): self.maxDiff = None EXPECTED = """usage: python -m pyt [-h] [-a ADAPTOR] [-pr PROJECT_ROOT] - [-b BASELINE_JSON_FILE] [-j] [-t TRIGGER_WORD_FILE] + [-b BASELINE_JSON_FILE] [-t TRIGGER_WORD_FILE] [-m BLACKBOX_MAPPING_FILE] [-i] [-o OUTPUT_FILE] [--ignore-nosec] [-r] [-x EXCLUDED_PATHS] - [--dont-prepend-root] [--no-local-imports] + [--dont-prepend-root] [--no-local-imports] [-u] [-j | -s] targets [targets ...] required arguments: @@ -45,7 +45,6 @@ def test_no_args(self): -b BASELINE_JSON_FILE, --baseline BASELINE_JSON_FILE Path of a baseline report to compare against (only JSON-formatted files are accepted) - -j, --json Prints JSON instead of report. -t TRIGGER_WORD_FILE, --trigger-word-file TRIGGER_WORD_FILE Input file with a list of sources and sinks -m BLACKBOX_MAPPING_FILE, --blackbox-mapping-file BLACKBOX_MAPPING_FILE @@ -62,7 +61,11 @@ def test_no_args(self): with app.* --no-local-imports If set, absolute imports must be relative to the project root. If not set, modules in the same - directory can be imported just by their names.\n""" + directory can be imported just by their names. + -u, --only-unsanitised + Don't print sanitised vulnerabilities. + -j, --json Prints JSON instead of report. + -s, --screen Prints colorful report.\n""" self.assertEqual(stdout.getvalue(), EXPECTED) @@ -72,10 +75,10 @@ def test_valid_args_but_no_targets(self): parse_args(['-j']) EXPECTED = """usage: python -m pyt [-h] [-a ADAPTOR] [-pr PROJECT_ROOT] - [-b BASELINE_JSON_FILE] [-j] [-t TRIGGER_WORD_FILE] + [-b BASELINE_JSON_FILE] [-t TRIGGER_WORD_FILE] [-m BLACKBOX_MAPPING_FILE] [-i] [-o OUTPUT_FILE] [--ignore-nosec] [-r] [-x EXCLUDED_PATHS] - [--dont-prepend-root] [--no-local-imports] + [--dont-prepend-root] [--no-local-imports] [-u] [-j | -s] targets [targets ...] python -m pyt: error: the following arguments are required: targets\n""" From bf4925901e1c7fafb9b5e329d69481baf6008c99 Mon Sep 17 00:00:00 2001 From: bcaller Date: Wed, 5 Sep 2018 16:29:30 +0100 Subject: [PATCH 4/4] Add basic python logging to pyt with -v -vv -vvv Very basic python logging added to pyt. Very useful when you want to see: - which files are being processed - if your imports are not being inspected - which file crashed pyt --- pyt/__main__.py | 14 ++++++++++++++ pyt/cfg/stmt_visitor.py | 21 ++++++++++++++++++--- pyt/core/ast_helper.py | 8 ++++---- pyt/usage.py | 6 +++++- tests/usage_test.py | 5 +++-- 5 files changed, 44 insertions(+), 10 deletions(-) diff --git a/pyt/__main__.py b/pyt/__main__.py index 3e6a9911..d952cbe3 100644 --- a/pyt/__main__.py +++ b/pyt/__main__.py @@ -1,5 +1,6 @@ """The comand line module of PyT.""" +import logging import os import sys from collections import defaultdict @@ -26,6 +27,8 @@ is_function_without_leading_ ) +log = logging.getLogger(__name__) + def discover_files(targets, excluded_files, recursive=False): included_files = list() @@ -37,11 +40,13 @@ def discover_files(targets, excluded_files, recursive=False): if file.endswith('.py') and file not in excluded_list: fullpath = os.path.join(root, file) included_files.append(fullpath) + log.debug('Discovered file: %s', fullpath) if not recursive: break else: if target not in excluded_list: included_files.append(target) + log.debug('Discovered file: %s', target) return included_files @@ -60,6 +65,14 @@ def retrieve_nosec_lines( def main(command_line_args=sys.argv[1:]): # noqa: C901 args = parse_args(command_line_args) + logging_level = ( + logging.ERROR if not args.verbose else + logging.WARN if args.verbose == 1 else + logging.INFO if args.verbose == 2 else + logging.DEBUG + ) + logging.basicConfig(level=logging_level, format='[%(levelname)s] %(name)s: %(message)s') + files = discover_files( args.targets, args.excluded_paths, @@ -74,6 +87,7 @@ def main(command_line_args=sys.argv[1:]): # noqa: C901 cfg_list = list() for path in sorted(files): + log.info("Processing %s", path) if not args.ignore_nosec: nosec_lines[path] = retrieve_nosec_lines(path) diff --git a/pyt/cfg/stmt_visitor.py b/pyt/cfg/stmt_visitor.py index ce4d198b..95913211 100644 --- a/pyt/cfg/stmt_visitor.py +++ b/pyt/cfg/stmt_visitor.py @@ -1,6 +1,8 @@ import ast import itertools +import logging import os.path +from pkgutil import iter_modules from .alias_helper import ( as_alias_handler, @@ -52,6 +54,9 @@ remove_breaks ) +log = logging.getLogger(__name__) +uninspectable_modules = {module.name for module in iter_modules()} # Don't warn about failing to import these + class StmtVisitor(ast.NodeVisitor): def __init__(self, allow_local_directory_imports=True): @@ -429,9 +434,12 @@ def visit_Assign(self, node): else: label = LabelVisitor() label.visit(node) - print('Assignment not properly handled.', - 'Could result in not finding a vulnerability.', - 'Assignment:', label.result) + log.warn( + 'Assignment not properly handled in %s. Could result in not finding a vulnerability.' + 'Assignment: %s', + getattr(self, 'filenames', ['?'])[0], + self.label.result, + ) return self.append_node(AssignmentNode( label.result, label.result, @@ -1022,6 +1030,10 @@ def visit_Import(self, node): name.asname, retrieve_import_alias_mapping(node.names) ) + for alias in node.names: + if alias.name not in uninspectable_modules: + log.warn("Cannot inspect module %s", alias.name) + uninspectable_modules.add(alias.name) # Don't repeatedly warn about this return IgnoredNode() def visit_ImportFrom(self, node): @@ -1061,4 +1073,7 @@ def visit_ImportFrom(self, node): retrieve_import_alias_mapping(node.names), from_from=True ) + if node.module not in uninspectable_modules: + log.warn("Cannot inspect module %s", node.module) + uninspectable_modules.add(node.module) return IgnoredNode() diff --git a/pyt/core/ast_helper.py b/pyt/core/ast_helper.py index 4ca1ca69..e4ccbca2 100644 --- a/pyt/core/ast_helper.py +++ b/pyt/core/ast_helper.py @@ -2,13 +2,14 @@ Useful when working with the ast module.""" import ast +import logging import os import subprocess from functools import lru_cache from .transformer import PytTransformer - +log = logging.getLogger(__name__) BLACK_LISTED_CALL_NAMES = ['self'] recursive = False @@ -16,11 +17,10 @@ def _convert_to_3(path): # pragma: no cover """Convert python 2 file to python 3.""" try: - print('##### Trying to convert file to Python 3. #####') + log.warn('##### Trying to convert %s to Python 3. #####', path) subprocess.call(['2to3', '-w', path]) except subprocess.SubprocessError: - print('Check if 2to3 is installed. ' - 'https://docs.python.org/2/library/2to3.html') + log.exception('Check if 2to3 is installed. https://docs.python.org/2/library/2to3.html') exit(1) diff --git a/pyt/usage.py b/pyt/usage.py index 7acc5be7..9de776cf 100644 --- a/pyt/usage.py +++ b/pyt/usage.py @@ -30,7 +30,11 @@ def _add_required_group(parser): def _add_optional_group(parser): optional_group = parser.add_argument_group('optional arguments') - + optional_group.add_argument( + '-v', '--verbose', + action='count', + help='Increase logging verbosity. Can repeated e.g. -vvv', + ) optional_group.add_argument( '-a', '--adaptor', help='Choose a web framework adaptor: ' diff --git a/tests/usage_test.py b/tests/usage_test.py index 294004a4..cf79f6c5 100644 --- a/tests/usage_test.py +++ b/tests/usage_test.py @@ -25,7 +25,7 @@ def test_no_args(self): self.maxDiff = None - EXPECTED = """usage: python -m pyt [-h] [-a ADAPTOR] [-pr PROJECT_ROOT] + EXPECTED = """usage: python -m pyt [-h] [-v] [-a ADAPTOR] [-pr PROJECT_ROOT] [-b BASELINE_JSON_FILE] [-t TRIGGER_WORD_FILE] [-m BLACKBOX_MAPPING_FILE] [-i] [-o OUTPUT_FILE] [--ignore-nosec] [-r] [-x EXCLUDED_PATHS] @@ -36,6 +36,7 @@ def test_no_args(self): targets source file(s) or directory(s) to be scanned optional arguments: + -v, --verbose Increase logging verbosity. Can repeated e.g. -vvv -a ADAPTOR, --adaptor ADAPTOR Choose a web framework adaptor: Flask(Default), Django, Every or Pylons @@ -74,7 +75,7 @@ def test_valid_args_but_no_targets(self): with capture_sys_output() as (_, stderr): parse_args(['-j']) - EXPECTED = """usage: python -m pyt [-h] [-a ADAPTOR] [-pr PROJECT_ROOT] + EXPECTED = """usage: python -m pyt [-h] [-v] [-a ADAPTOR] [-pr PROJECT_ROOT] [-b BASELINE_JSON_FILE] [-t TRIGGER_WORD_FILE] [-m BLACKBOX_MAPPING_FILE] [-i] [-o OUTPUT_FILE] [--ignore-nosec] [-r] [-x EXCLUDED_PATHS]

AltStyle によって変換されたページ (->オリジナル) /