From 054b70611579d28e6dcef4c9e1c4356e42a698d5 Mon Sep 17 00:00:00 2001 From: Kevin Hock Date: Wed, 18 Apr 2018 19:14:55 -0700 Subject: [PATCH] Skip making self a TaintedNode, vuln test for it, flake8 vuln_test, 88->89% --- .pre-commit-config.yaml | 2 +- .../def_with_self_as_first_arg.py | 4 ++ pyt/framework_adaptor.py | 31 +++++++++---- pyt/node_types.py | 4 ++ tests/vulnerabilities_test.py | 46 ++++++++++++++++--- tox.ini | 2 +- 6 files changed, 70 insertions(+), 19 deletions(-) create mode 100644 examples/example_inputs/def_with_self_as_first_arg.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8ef0f176..33a48e76 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -8,4 +8,4 @@ - id: check-ast - id: check-symlinks - id: flake8 - args: ['--ignore=E501'] + args: ['--exclude=examples/*', '--ignore=E501,E741'] diff --git a/examples/example_inputs/def_with_self_as_first_arg.py b/examples/example_inputs/def_with_self_as_first_arg.py new file mode 100644 index 00000000..0c244455 --- /dev/null +++ b/examples/example_inputs/def_with_self_as_first_arg.py @@ -0,0 +1,4 @@ + + +def my_data(self, foo, bar): + return redirect(self.something) diff --git a/pyt/framework_adaptor.py b/pyt/framework_adaptor.py index d602a92f..c7e5119d 100644 --- a/pyt/framework_adaptor.py +++ b/pyt/framework_adaptor.py @@ -5,7 +5,10 @@ from .ast_helper import Arguments from .expr_visitor import make_cfg from .module_definitions import project_definitions -from .node_types import TaintedNode +from .node_types import ( + AssignmentNode, + TaintedNode +) class FrameworkAdaptor(): @@ -37,19 +40,27 @@ def get_func_cfg_with_tainted_args(self, definition): first_node_after_args = func_cfg.nodes[1] first_node_after_args.ingoing = list() - # We're just gonna give all the tainted args the lineno of the def + # We are just going to give all the tainted args the lineno of the def definition_lineno = definition.node.lineno # Taint all the arguments - for arg in args: - tainted_node = TaintedNode(arg, arg, - None, [], - line_number=definition_lineno, - path=definition.path) - function_entry_node.connect(tainted_node) + for i, arg in enumerate(args): + node_type = TaintedNode + if i == 0 and arg == 'self': + node_type = AssignmentNode + + arg_node = node_type( + label=arg, + left_hand_side=arg, + ast_node=None, + right_hand_side_variables=[], + line_number=definition_lineno, + path=definition.path + ) + function_entry_node.connect(arg_node) # 1 and not 0 so that Entry Node remains first in the list - func_cfg.nodes.insert(1, tainted_node) - tainted_node.connect(first_node_after_args) + func_cfg.nodes.insert(1, arg_node) + arg_node.connect(first_node_after_args) return func_cfg diff --git a/pyt/node_types.py b/pyt/node_types.py index 9d194bcf..3819963a 100644 --- a/pyt/node_types.py +++ b/pyt/node_types.py @@ -176,6 +176,10 @@ def __repr__(self): class TaintedNode(AssignmentNode): + """CFG Node that represents a tainted node. + + Only created in framework_adaptor.py and only used in `identify_triggers` of vulnerabilities.py + """ pass diff --git a/tests/vulnerabilities_test.py b/tests/vulnerabilities_test.py index fb3c00f2..f3d77279 100644 --- a/tests/vulnerabilities_test.py +++ b/tests/vulnerabilities_test.py @@ -15,9 +15,10 @@ from pyt.constraint_table import initialize_constraint_table from pyt.fixed_point import analyse from pyt.framework_adaptor import FrameworkAdaptor -from pyt.framework_helper import( +from pyt.framework_helper import ( is_django_view_function, - is_flask_route_function + is_flask_route_function, + is_function ) from pyt.node_types import Node from pyt.reaching_definitions_taint import ReachingDefinitionsTaintAnalysis @@ -95,17 +96,15 @@ def test_find_triggers(self): l = vulnerabilities.find_triggers(XSS1.nodes, trigger_words) self.assert_length(l, expected_length=1) - def test_find_sanitiser_nodes(self): cfg_node = Node(None, None, line_number=None, path=None) - sanitiser_tuple = vulnerabilities.Sanitiser('escape', cfg_node) + sanitiser_tuple = vulnerabilities.Sanitiser('escape', cfg_node) sanitiser = 'escape' result = list(vulnerabilities.find_sanitiser_nodes(sanitiser, [sanitiser_tuple])) self.assert_length(result, expected_length=1) self.assertEqual(result[0], cfg_node) - def test_build_sanitiser_node_dict(self): self.cfg_create_from_file('examples/vulnerable_code/XSS_sanitised.py') cfg_list = [self.cfg] @@ -114,7 +113,7 @@ def test_build_sanitiser_node_dict(self): cfg = cfg_list[1] - cfg_node = Node(None, None, line_number=None, path=None) + cfg_node = Node(None, None, line_number=None, path=None) sinks_in_file = [vulnerabilities.TriggerNode('replace', ['escape'], cfg_node)] sanitiser_dict = vulnerabilities.build_sanitiser_node_dict(cfg, sinks_in_file) @@ -142,7 +141,6 @@ def run_analysis(self, path): ) ) - def test_find_vulnerabilities_assign_other_var(self): vulnerabilities = self.run_analysis('examples/vulnerable_code/XSS_assign_to_other_var.py') self.assert_length(vulnerabilities, expected_length=1) @@ -555,3 +553,37 @@ def test_django_view_param(self): ~call_1 = ret_render(request, 'templates/xss.html', 'param'param) """ self.assertTrue(self.string_compare_alpha(vulnerability_description, EXPECTED_VULNERABILITY_DESCRIPTION)) + + +class EngineEveryTest(BaseTestCase): + def run_empty(self): + return + + def run_analysis(self, path): + self.cfg_create_from_file(path) + cfg_list = [self.cfg] + + FrameworkAdaptor(cfg_list, [], [], is_function) + initialize_constraint_table(cfg_list) + + analyse(cfg_list, analysis_type=ReachingDefinitionsTaintAnalysis) + + trigger_word_file = os.path.join( + 'pyt', + 'vulnerability_definitions', + 'all_trigger_words.pyt' + ) + + return vulnerabilities.find_vulnerabilities( + cfg_list, + ReachingDefinitionsTaintAnalysis, + UImode.NORMAL, + VulnerabilityFiles( + default_blackbox_mapping_file, + trigger_word_file + ) + ) + + def test_self_is_not_tainted(self): + vulnerabilities = self.run_analysis('examples/example_inputs/def_with_self_as_first_arg.py') + self.assert_length(vulnerabilities, expected_length=0) diff --git a/tox.ini b/tox.ini index 933a1460..70393146 100644 --- a/tox.ini +++ b/tox.ini @@ -7,5 +7,5 @@ deps = -rrequirements-dev.txt commands = coverage erase coverage run tests - coverage report --show-missing --fail-under 88 + coverage report --show-missing --fail-under 89 pre-commit run