From e10f850bd879b8f5c1627919410e369164423583 Mon Sep 17 00:00:00 2001 From: KevinHock Date: Thu, 8 Nov 2018 17:08:24 -0800 Subject: [PATCH 01/27] Add link to AMF Ranked #1 on [Givewell](https://www.givewell.org/charities/top-charities) --- README.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.rst b/README.rst index ad79c794..80ac92a5 100644 --- a/README.rst +++ b/README.rst @@ -16,6 +16,9 @@ .. image:: https://img.shields.io/badge/python-v3.6-blue.svg :target: https://pypi.org/project/python-taint/ +.. image:: https://img.shields.io/badge/Donate-Charity-orange.svg + :target: https://www.againstmalaria.com/donation.aspx + Python Taint ============ From ce56a20731de1b6245fd76555bdab00fc9fa07fb Mon Sep 17 00:00:00 2001 From: KevinHock Date: Thu, 8 Nov 2018 17:52:11 -0800 Subject: [PATCH 02/27] Fix link for PRs Welcome Badge --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 80ac92a5..025a3705 100644 --- a/README.rst +++ b/README.rst @@ -11,7 +11,7 @@ :target: https://badge.fury.io/py/python-taint .. image:: https://img.shields.io/badge/PRs-welcome-ff69b4.svg - :target: https://github.com/python-security/pyt/issues?q=is%3Aopen+is%3Aissue+label%3Agood-first-issue + :target: https://github.com/python-security/pyt/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22+ .. image:: https://img.shields.io/badge/python-v3.6-blue.svg :target: https://pypi.org/project/python-taint/ From e704c21116bd9248312a54a5aeed77634fb22345 Mon Sep 17 00:00:00 2001 From: Adrian Bravo Date: Mon, 19 Nov 2018 16:05:47 -0800 Subject: [PATCH 03/27] 133: Visit functions in while test --- .../example_inputs/while_func_comparator.py | 6 ++++ pyt/cfg/stmt_visitor.py | 9 ++++-- tests/cfg/cfg_test.py | 32 +++++++++++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 examples/example_inputs/while_func_comparator.py diff --git a/examples/example_inputs/while_func_comparator.py b/examples/example_inputs/while_func_comparator.py new file mode 100644 index 00000000..6aafc2b6 --- /dev/null +++ b/examples/example_inputs/while_func_comparator.py @@ -0,0 +1,6 @@ +def foo(): + return 6 + +while x < foo(): + print(x) + x += 1 diff --git a/pyt/cfg/stmt_visitor.py b/pyt/cfg/stmt_visitor.py index 95913211..ab45707a 100644 --- a/pyt/cfg/stmt_visitor.py +++ b/pyt/cfg/stmt_visitor.py @@ -565,13 +565,18 @@ def visit_While(self, node): label_visitor = LabelVisitor() label_visitor.visit(node.test) - test = self.append_node(Node( + while_node = self.append_node(Node( 'while ' + label_visitor.result + ':', node, path=self.filenames[-1] )) - return self.loop_node_skeleton(test, node) + for comp in node.test.comparators: + if isinstance(comp, ast.Call) and get_call_names_as_string(comp.func) in self.function_names: + last_node = self.visit(comp) + last_node.connect(while_node) + + return self.loop_node_skeleton(while_node, node) def add_blackbox_or_builtin_call(self, node, blackbox): # noqa: C901 """Processes a blackbox or builtin function when it is called. diff --git a/tests/cfg/cfg_test.py b/tests/cfg/cfg_test.py index a4c24ba5..cfdc056a 100644 --- a/tests/cfg/cfg_test.py +++ b/tests/cfg/cfg_test.py @@ -684,6 +684,38 @@ def test_while_line_numbers(self): self.assertLineNumber(else_body_2, 6) self.assertLineNumber(next_stmt, 7) + def test_while_func_iterator(self): + self.cfg_create_from_file('examples/example_inputs/while_func_comparator.py') + + self.assert_length(self.cfg.nodes, expected_length=9) + + entry = 0 + test = 1 + entry_foo = 2 + ret_foo = 3 + exit_foo = 4 + call_foo = 5 + _print = 6 + body_1 = 7 + _exit = 8 + + self.assertEqual(self.cfg.nodes[test].label, 'while x < foo():') + + self.assertInCfg([ + (test, entry), + (entry_foo, test), + (_print, test), + (_exit, test), + (body_1, _print), + + (test, body_1), + (test, call_foo), + (ret_foo, entry_foo), + (exit_foo, ret_foo), + (call_foo, exit_foo), + + ]) + class CFGAssignmentMultiTest(CFGBaseTestCase): def test_assignment_multi_target(self): From effd87248cfba31318d61c72b3a0862e5313345a Mon Sep 17 00:00:00 2001 From: Adrian Bravo Date: Tue, 20 Nov 2018 10:03:54 -0800 Subject: [PATCH 04/27] 133: Support for LHS functions and no comparison while tests --- .../example_inputs/while_func_comparator.py | 4 +- .../while_func_comparator_lhs.py | 6 +++ .../while_func_comparator_rhs.py | 6 +++ pyt/cfg/stmt_visitor.py | 18 +++++-- tests/cfg/cfg_test.py | 51 +++++++++++++++++-- 5 files changed, 76 insertions(+), 9 deletions(-) create mode 100644 examples/example_inputs/while_func_comparator_lhs.py create mode 100644 examples/example_inputs/while_func_comparator_rhs.py diff --git a/examples/example_inputs/while_func_comparator.py b/examples/example_inputs/while_func_comparator.py index 6aafc2b6..8c775f72 100644 --- a/examples/example_inputs/while_func_comparator.py +++ b/examples/example_inputs/while_func_comparator.py @@ -1,6 +1,6 @@ def foo(): - return 6 + return True -while x < foo(): +while foo(): print(x) x += 1 diff --git a/examples/example_inputs/while_func_comparator_lhs.py b/examples/example_inputs/while_func_comparator_lhs.py new file mode 100644 index 00000000..1904e8e7 --- /dev/null +++ b/examples/example_inputs/while_func_comparator_lhs.py @@ -0,0 +1,6 @@ +def foo(): + return 6 + +while foo() > x: + print(x) + x += 1 diff --git a/examples/example_inputs/while_func_comparator_rhs.py b/examples/example_inputs/while_func_comparator_rhs.py new file mode 100644 index 00000000..6aafc2b6 --- /dev/null +++ b/examples/example_inputs/while_func_comparator_rhs.py @@ -0,0 +1,6 @@ +def foo(): + return 6 + +while x < foo(): + print(x) + x += 1 diff --git a/pyt/cfg/stmt_visitor.py b/pyt/cfg/stmt_visitor.py index ab45707a..965b24eb 100644 --- a/pyt/cfg/stmt_visitor.py +++ b/pyt/cfg/stmt_visitor.py @@ -563,7 +563,8 @@ def visit_For(self, node): def visit_While(self, node): label_visitor = LabelVisitor() - label_visitor.visit(node.test) + test = node.test # the test condition of the while loop + label_visitor.visit(test) while_node = self.append_node(Node( 'while ' + label_visitor.result + ':', @@ -571,11 +572,20 @@ def visit_While(self, node): path=self.filenames[-1] )) - for comp in node.test.comparators: - if isinstance(comp, ast.Call) and get_call_names_as_string(comp.func) in self.function_names: - last_node = self.visit(comp) + def process_comparator(comp_n): + if isinstance(comp_n, ast.Call) and get_call_names_as_string(comp_n.func) in self.function_names: + last_node = self.visit(comp_n) last_node.connect(while_node) + if isinstance(test, ast.Compare): + comparators = test.comparators + comparators.append(test.left) # quirk. See https://greentreesnakes.readthedocs.io/en/latest/nodes.html#Compare + + for comp in comparators: + process_comparator(comp) + else: # while foo(): + process_comparator(test) + return self.loop_node_skeleton(while_node, node) def add_blackbox_or_builtin_call(self, node, blackbox): # noqa: C901 diff --git a/tests/cfg/cfg_test.py b/tests/cfg/cfg_test.py index cfdc056a..bf47275e 100644 --- a/tests/cfg/cfg_test.py +++ b/tests/cfg/cfg_test.py @@ -684,7 +684,7 @@ def test_while_line_numbers(self): self.assertLineNumber(else_body_2, 6) self.assertLineNumber(next_stmt, 7) - def test_while_func_iterator(self): + def test_while_func_comparator(self): self.cfg_create_from_file('examples/example_inputs/while_func_comparator.py') self.assert_length(self.cfg.nodes, expected_length=9) @@ -699,6 +699,23 @@ def test_while_func_iterator(self): body_1 = 7 _exit = 8 + self.assertEqual(self.cfg.nodes[test].label, 'while foo():') + + def test_while_func_comparator_rhs(self): + self.cfg_create_from_file('examples/example_inputs/while_func_comparator_rhs.py') + + self.assert_length(self.cfg.nodes, expected_length=9) + + entry = 0 + test = 1 + entry_foo = 2 + ret_foo = 3 + exit_foo = 4 + call_foo = 5 + _print = 6 + body_1 = 7 + _exit = 8 + self.assertEqual(self.cfg.nodes[test].label, 'while x < foo():') self.assertInCfg([ @@ -707,13 +724,41 @@ def test_while_func_iterator(self): (_print, test), (_exit, test), (body_1, _print), - (test, body_1), (test, call_foo), (ret_foo, entry_foo), (exit_foo, ret_foo), - (call_foo, exit_foo), + (call_foo, exit_foo) + ]) + + def test_while_func_comparator_lhs(self): + self.cfg_create_from_file('examples/example_inputs/while_func_comparator_lhs.py') + self.assert_length(self.cfg.nodes, expected_length=9) + + entry = 0 + test = 1 + entry_foo = 2 + ret_foo = 3 + exit_foo = 4 + call_foo = 5 + _print = 6 + body_1 = 7 + _exit = 8 + + self.assertEqual(self.cfg.nodes[test].label, 'while foo() > x:') + + self.assertInCfg([ + (test, entry), + (entry_foo, test), + (_print, test), + (_exit, test), + (body_1, _print), + (test, body_1), + (test, call_foo), + (ret_foo, entry_foo), + (exit_foo, ret_foo), + (call_foo, exit_foo) ]) From b52a8707b7fc598d1c7f59c785be0269d1b4a1d5 Mon Sep 17 00:00:00 2001 From: Adrian Bravo Date: Tue, 20 Nov 2018 10:17:20 -0800 Subject: [PATCH 05/27] 133: Fix style and complexity issues for Travis.ci --- pyt/cfg/stmt_visitor.py | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/pyt/cfg/stmt_visitor.py b/pyt/cfg/stmt_visitor.py index 965b24eb..d3f8e2c8 100644 --- a/pyt/cfg/stmt_visitor.py +++ b/pyt/cfg/stmt_visitor.py @@ -555,15 +555,26 @@ def visit_For(self, node): path=self.filenames[-1] )) - if isinstance(node.iter, ast.Call) and get_call_names_as_string(node.iter.func) in self.function_names: - last_node = self.visit(node.iter) - last_node.connect(for_node) + self.process_loop_funcs(node.iter, for_node) return self.loop_node_skeleton(for_node, node) + def process_loop_funcs(self, comp_n, loop_node): + """ + If the loop test node contains function calls, it connects the loop node to the nodes of + those function calls. + + :param comp_n: The test node of a loop that may contain functions. + :param loop_node: The loop node itself to connect to the new function nodes if any + :return: None + """ + if isinstance(comp_n, ast.Call) and get_call_names_as_string(comp_n.func) in self.function_names: + last_node = self.visit(comp_n) + last_node.connect(loop_node) + def visit_While(self, node): label_visitor = LabelVisitor() - test = node.test # the test condition of the while loop + test = node.test # the test condition of the while loop label_visitor.visit(test) while_node = self.append_node(Node( @@ -572,19 +583,14 @@ def visit_While(self, node): path=self.filenames[-1] )) - def process_comparator(comp_n): - if isinstance(comp_n, ast.Call) and get_call_names_as_string(comp_n.func) in self.function_names: - last_node = self.visit(comp_n) - last_node.connect(while_node) - if isinstance(test, ast.Compare): comparators = test.comparators - comparators.append(test.left) # quirk. See https://greentreesnakes.readthedocs.io/en/latest/nodes.html#Compare + comparators.append(test.left) # quirk. See https://greentreesnakes.readthedocs.io/en/latest/nodes.html#Compare for comp in comparators: - process_comparator(comp) - else: # while foo(): - process_comparator(test) + self.process_loop_funcs(comp, while_node) + else: # while foo(): + self.process_loop_funcs(test, while_node) return self.loop_node_skeleton(while_node, node) From 6d25eae6ce969e1930846e6714445ee5ca870a47 Mon Sep 17 00:00:00 2001 From: Adrian Bravo Date: Tue, 20 Nov 2018 10:21:56 -0800 Subject: [PATCH 06/27] 133: Finished tests --- tests/cfg/cfg_test.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/cfg/cfg_test.py b/tests/cfg/cfg_test.py index bf47275e..3af37942 100644 --- a/tests/cfg/cfg_test.py +++ b/tests/cfg/cfg_test.py @@ -701,6 +701,19 @@ def test_while_func_comparator(self): self.assertEqual(self.cfg.nodes[test].label, 'while foo():') + self.assertInCfg([ + (test, entry), + (entry_foo, test), + (_print, test), + (_exit, test), + (body_1, _print), + (test, body_1), + (test, call_foo), + (ret_foo, entry_foo), + (exit_foo, ret_foo), + (call_foo, exit_foo) + ]) + def test_while_func_comparator_rhs(self): self.cfg_create_from_file('examples/example_inputs/while_func_comparator_rhs.py') From 9cb0b567c8cd0c66bcf9ac87ad1425d47a75b908 Mon Sep 17 00:00:00 2001 From: Adrian Bravo Date: Wed, 21 Nov 2018 10:12:19 -0800 Subject: [PATCH 07/27] 133: Avoid mutating node.test.comparators --- pyt/cfg/stmt_visitor.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pyt/cfg/stmt_visitor.py b/pyt/cfg/stmt_visitor.py index d3f8e2c8..3b9d5f48 100644 --- a/pyt/cfg/stmt_visitor.py +++ b/pyt/cfg/stmt_visitor.py @@ -584,10 +584,10 @@ def visit_While(self, node): )) if isinstance(test, ast.Compare): - comparators = test.comparators - comparators.append(test.left) # quirk. See https://greentreesnakes.readthedocs.io/en/latest/nodes.html#Compare + # quirk. See https://greentreesnakes.readthedocs.io/en/latest/nodes.html#Compare + self.process_loop_funcs(test.left, while_node) - for comp in comparators: + for comp in test.comparators: self.process_loop_funcs(comp, while_node) else: # while foo(): self.process_loop_funcs(test, while_node) From aff6b6c166e73c7c4ad4f9a1d863a6fad28cc785 Mon Sep 17 00:00:00 2001 From: Kevin Hock Date: Sat, 24 Nov 2018 14:15:48 -0800 Subject: [PATCH 08/27] [spelling] constaint -> constraint --- pyt/vulnerabilities/vulnerabilities.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/pyt/vulnerabilities/vulnerabilities.py b/pyt/vulnerabilities/vulnerabilities.py index 0daca2cd..3087c1bd 100644 --- a/pyt/vulnerabilities/vulnerabilities.py +++ b/pyt/vulnerabilities/vulnerabilities.py @@ -359,12 +359,12 @@ def how_vulnerable( def get_tainted_node_in_sink_args( sink_args, - nodes_in_constaint + nodes_in_constraint ): if not sink_args: return None # Starts with the node closest to the sink - for node in nodes_in_constaint: + for node in nodes_in_constraint: if node.left_hand_side in sink_args: return node @@ -398,11 +398,10 @@ def get_vulnerability( Returns: A Vulnerability if it exists, else None """ - nodes_in_constaint = [secondary for secondary in reversed(source.secondary_nodes) + nodes_in_constraint = [secondary for secondary in reversed(source.secondary_nodes) if lattice.in_constraint(secondary, sink.cfg_node)] - nodes_in_constaint.append(source.cfg_node) - + nodes_in_constraint.append(source.cfg_node) if sink.trigger.all_arguments_propagate_taint: sink_args = get_sink_args(sink.cfg_node) else: @@ -410,7 +409,7 @@ def get_vulnerability( tainted_node_in_sink_arg = get_tainted_node_in_sink_args( sink_args, - nodes_in_constaint, + nodes_in_constraint, ) if tainted_node_in_sink_arg: From 6641078459411ff7b4c525c5aa3908d39ed539c7 Mon Sep 17 00:00:00 2001 From: KevinHock Date: Sat, 24 Nov 2018 14:40:53 -0800 Subject: [PATCH 09/27] Added links --- CHANGELOG.md | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 928f88cf..9c68d4c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,35 +26,47 @@ If you love PyT, please star our project on GitHub to show your support! :star: ##### November 1st, 2018 #### :boom: Breaking Changes -* Changed trigger file format when specifying specific tainted args ([#182]) +* Changed trigger file format when specifying specific tainted args ([#182], thanks [@bcaller]) #### :tada: New Features -* Function calls such as `list.append` and `dict.update` now propagate taint to the list or dict ([#181]) +* Function calls such as `list.append` and `dict.update` now propagate taint to the list or dict ([#181], thanks [@bcaller]) #### :bug: Bugfixes -* IfExp (or ternary) expression handling improved ([#179]) +* IfExp (or ternary) expression handling improved ([#179], thanks [@bcaller]) + +[#179]: https://github.com/python-security/pyt/pull/179 +[#181]: https://github.com/python-security/pyt/pull/181 +[#182]: https://github.com/python-security/pyt/pull/182 + # 0.40 ##### September 11th, 2018 #### :mega: Release Highlights -* Logging changes. Logging verbosity can be changed with `-v` to `-vvv` ([#172]) +* Logging changes. Logging verbosity can be changed with `-v` to `-vvv` ([#172], thanks [@bcaller]) #### :boom: Breaking Changes * Removed `--trim` option ([#169]) #### :tada: New Features -* Added `--only-unsanitised` flag to not print sanitised vulnerabilities ([#172]) +* Added `--only-unsanitised` flag to not print sanitised vulnerabilities ([#172], thanks [@bcaller]) #### :bug: Bugfixes -* Recursive functions don't cause `RecursionError` ([#173]) -* Handling of chained functions improved ([#171]) +* Recursive functions don't cause `RecursionError` ([#173], thanks [@bcaller]) +* Handling of chained functions improved ([#171], thanks [@bcaller]) + +[#169]: https://github.com/python-security/pyt/pull/169 +[#171]: https://github.com/python-security/pyt/pull/171 +[#172]: https://github.com/python-security/pyt/pull/172 +[#173]: https://github.com/python-security/pyt/pull/173 + # 0.39 ##### August 21st, 2018 ... + # 0.38 ##### August 2nd, 2018 From 52cac9d376448003737e8ffdc7560cf99b4d1758 Mon Sep 17 00:00:00 2001 From: KevinHock Date: Sat, 24 Nov 2018 14:47:10 -0800 Subject: [PATCH 10/27] Added most recent PR details --- CHANGELOG.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c68d4c1..36909cfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,17 @@ If you love PyT, please star our project on GitHub to show your support! :star: [#xxxx]: https://github.com/python-security/pyt/pull/xxxx [@xxxx]: https://github.com/xxxx --> + +# Unreleased + +#### :tada: New Features + +* Added visting functions in the tests of `while` nodes ([#186], thanks [@adrianbn]) + +[@adrianbn]: https://github.com/adrianbn +[#186]: https://github.com/python-security/pyt/pull/186 + + # 0.42 ##### November 1st, 2018 @@ -156,6 +167,7 @@ If you love PyT, please star our project on GitHub to show your support! :star: [#152]: https://github.com/python-security/pyt/pull/152 [#156]: https://github.com/python-security/pyt/pull/156 + # 0.34 ##### April 24th, 2018 From 95e2ac3bdb02647f068f8fdc8a2aaad1fe67e0da Mon Sep 17 00:00:00 2001 From: KevinHock Date: Sat, 24 Nov 2018 15:02:09 -0800 Subject: [PATCH 11/27] Added older `0.39` version details --- CHANGELOG.md | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36909cfc..3d626450 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,7 +75,27 @@ If you love PyT, please star our project on GitHub to show your support! :star: # 0.39 ##### August 21st, 2018 -... +#### :tada: New Features + +* Added handling of assignment unpacking e.g. `a, b, c = d` ([#164], thanks [@bcaller]) +* Made file loading and vulnerability order deterministic ([#165], thanks [@bcaller]) + +#### :bug: Bugfixes +* Fixed VarsVisitor RuntimeError on code like `f(g(a)(b)(c))` ([#163], thanks [@bcaller]) + +#### :telescope: Precision + +* Taint propagates from methods of tainted objects ([#167], thanks [@bcaller]) + +#### :snake: Miscellaneous + +* Cleaned test cases of extraneous reassignments ([#166], thanks [@bcaller]) + +[#163]: https://github.com/python-security/pyt/pull/163 +[#164]: https://github.com/python-security/pyt/pull/164 +[#165]: https://github.com/python-security/pyt/pull/165 +[#166]: https://github.com/python-security/pyt/pull/166 +[#167]: https://github.com/python-security/pyt/pull/167 # 0.38 From 1b61080cbf83979194e32434396d72f122a87650 Mon Sep 17 00:00:00 2001 From: KevinHock Date: Sun, 25 Nov 2018 14:49:17 -0800 Subject: [PATCH 12/27] :bug: Fix E128 Flake8 Tavis Failure: `continuation line under-indented for visual indent` --- pyt/vulnerabilities/vulnerabilities.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pyt/vulnerabilities/vulnerabilities.py b/pyt/vulnerabilities/vulnerabilities.py index 3087c1bd..86b3a402 100644 --- a/pyt/vulnerabilities/vulnerabilities.py +++ b/pyt/vulnerabilities/vulnerabilities.py @@ -398,9 +398,14 @@ def get_vulnerability( Returns: A Vulnerability if it exists, else None """ - nodes_in_constraint = [secondary for secondary in reversed(source.secondary_nodes) - if lattice.in_constraint(secondary, - sink.cfg_node)] + nodes_in_constraint = [ + secondary + for secondary in reversed(source.secondary_nodes) + if lattice.in_constraint( + secondary, + sink.cfg_node + ) + ] nodes_in_constraint.append(source.cfg_node) if sink.trigger.all_arguments_propagate_taint: sink_args = get_sink_args(sink.cfg_node) From c0ef67500dff19f07a0251863d7b83dc6a23bf78 Mon Sep 17 00:00:00 2001 From: Adrian Bravo Date: Sat, 8 Dec 2018 15:16:44 +0100 Subject: [PATCH 13/27] 128: Allow the user to cancel interactive mode --- pyt/vulnerabilities/vulnerabilities.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pyt/vulnerabilities/vulnerabilities.py b/pyt/vulnerabilities/vulnerabilities.py index 86b3a402..a12d662c 100644 --- a/pyt/vulnerabilities/vulnerabilities.py +++ b/pyt/vulnerabilities/vulnerabilities.py @@ -336,11 +336,15 @@ def how_vulnerable( return VulnerabilityType.FALSE elif interactive: user_says = input( - 'Is the return value of {} with tainted argument "{}" vulnerable? (Y/n)'.format( + 'Is the return value of {} with tainted argument "{}" vulnerable? ([Y]es/[N]o/[S]top)'.format( current_node.label, chain[i - 1].left_hand_side ) ).lower() + if user_says.startswith('s'): + interactive = False + vuln_deets['unknown_assignment'] = current_node + return VulnerabilityType.UNKNOWN if user_says.startswith('n'): blackbox_mapping['does_not_propagate'].append(current_node.func_name) return VulnerabilityType.FALSE From 986532afdf4431610d9307c4d00a469cb3faac6d Mon Sep 17 00:00:00 2001 From: Adrian Bravo Date: Sun, 9 Dec 2018 01:04:52 +0100 Subject: [PATCH 14/27] 128: Stop asking for all chains --- pyt/vulnerabilities/vulnerabilities.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/pyt/vulnerabilities/vulnerabilities.py b/pyt/vulnerabilities/vulnerabilities.py index a12d662c..2dba0078 100644 --- a/pyt/vulnerabilities/vulnerabilities.py +++ b/pyt/vulnerabilities/vulnerabilities.py @@ -327,16 +327,16 @@ def how_vulnerable( if current_node in sanitiser_nodes: vuln_deets['sanitiser'] = current_node vuln_deets['confident'] = True - return VulnerabilityType.SANITISED + return VulnerabilityType.SANITISED, interactive if isinstance(current_node, BBorBInode): if current_node.func_name in blackbox_mapping['propagates']: continue elif current_node.func_name in blackbox_mapping['does_not_propagate']: - return VulnerabilityType.FALSE + return VulnerabilityType.FALSE, interactive elif interactive: user_says = input( - 'Is the return value of {} with tainted argument "{}" vulnerable? ([Y]es/[N]o/[S]top)'.format( + 'Is the return value of {} with tainted argument "{}" vulnerable? ([Y]es/[N]o/[S]top asking)'.format( current_node.label, chain[i - 1].left_hand_side ) @@ -344,21 +344,21 @@ def how_vulnerable( if user_says.startswith('s'): interactive = False vuln_deets['unknown_assignment'] = current_node - return VulnerabilityType.UNKNOWN + return VulnerabilityType.UNKNOWN, interactive if user_says.startswith('n'): blackbox_mapping['does_not_propagate'].append(current_node.func_name) - return VulnerabilityType.FALSE + return VulnerabilityType.FALSE, interactive blackbox_mapping['propagates'].append(current_node.func_name) else: vuln_deets['unknown_assignment'] = current_node - return VulnerabilityType.UNKNOWN + return VulnerabilityType.UNKNOWN, interactive if potential_sanitiser: vuln_deets['sanitiser'] = potential_sanitiser vuln_deets['confident'] = False - return VulnerabilityType.SANITISED + return VulnerabilityType.SANITISED, interactive - return VulnerabilityType.TRUE + return VulnerabilityType.TRUE, interactive def get_tainted_node_in_sink_args( @@ -443,12 +443,13 @@ def get_vulnerability( cfg.nodes, lattice ) + for chain in get_vulnerability_chains( source.cfg_node, sink.cfg_node, def_use ): - vulnerability_type = how_vulnerable( + vulnerability_type, interactive = how_vulnerable( chain, blackbox_mapping, sanitiser_nodes, @@ -462,9 +463,9 @@ def get_vulnerability( vuln_deets['reassignment_nodes'] = chain - return vuln_factory(vulnerability_type)(**vuln_deets) + return vuln_factory(vulnerability_type)(**vuln_deets), interactive - return None + return None, interactive def find_vulnerabilities_in_cfg( @@ -495,7 +496,7 @@ def find_vulnerabilities_in_cfg( ) for sink in triggers.sinks: for source in triggers.sources: - vulnerability = get_vulnerability( + vulnerability, interactive = get_vulnerability( source, sink, triggers, From 1332b7aa5942d63e8a82a4037682daa9dc206e9f Mon Sep 17 00:00:00 2001 From: bcaller Date: Wed, 16 Jan 2019 16:52:38 +0000 Subject: [PATCH 15/27] Test running pyt in a python 3.7 env It should just work. There were no AST changes. Also removes the unnecesary whitelist_external from tox.ini. Note: pyt can be run in a py36 environment on code targeting 2.7-3.8. There is no requirement for pyt to be run under the same environment as your code is intended to run in. Running pyt under 3.8 will require some further work due to the change from ast.Str to ast.Constant: https://github.com/python/cpython/pull/9445/files https://docs.python.org/dev/whatsnew/3.8.html#deprecated https://bugs.python.org/issue32892 --- .travis.yml | 2 ++ tox.ini | 3 +-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 0638ada6..fe655eef 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,6 +1,8 @@ language: python +dist: xenial python: - "3.6" + - "3.7" install: - pip install codeclimate-test-reporter 'coverage>=4.0,<4.4' flake8 before_script: diff --git a/tox.ini b/tox.ini index c9b4b248..424c51ef 100644 --- a/tox.ini +++ b/tox.ini @@ -1,12 +1,11 @@ [tox] -envlist = py36,cover,lint +envlist = py36,py37,cover,lint [testenv] commands = python -m tests [testenv:cover] -whitelist_externals = coverage deps = coverage>=4.0,<4.4 commands = From 40c1f2482dbc54b8846f22aae4082ebec6af5683 Mon Sep 17 00:00:00 2001 From: wchresta <34962284+wchresta@users.noreply.github.com> Date: Fri, 22 Mar 2019 17:01:05 -0400 Subject: [PATCH 16/27] Resolve aliases for black box and built-in function calls. * Allow trigger words to be fully qualified to reduce false positives --- .../command_injection_with_aliases.py | 12 +++++++++ pyt/cfg/alias_helper.py | 19 ++++++++++++++ pyt/cfg/stmt_visitor.py | 26 ++++++++++++++++--- .../all_trigger_words.pyt | 6 ++--- tests/main_test.py | 4 +-- tests/vulnerabilities/vulnerabilities_test.py | 15 +++++++++++ 6 files changed, 73 insertions(+), 9 deletions(-) create mode 100644 examples/vulnerable_code/command_injection_with_aliases.py diff --git a/examples/vulnerable_code/command_injection_with_aliases.py b/examples/vulnerable_code/command_injection_with_aliases.py new file mode 100644 index 00000000..309e5268 --- /dev/null +++ b/examples/vulnerable_code/command_injection_with_aliases.py @@ -0,0 +1,12 @@ +import os +import os as myos +from os import system +from os import system as mysystem +from subprocess import call as mycall, Popen as mypopen + +os.system("ls") +myos.system("ls") +system("ls") +mysystem("ls") +mycall("ls") +mypopen("ls") diff --git a/pyt/cfg/alias_helper.py b/pyt/cfg/alias_helper.py index a1c83ab0..9de6d058 100644 --- a/pyt/cfg/alias_helper.py +++ b/pyt/cfg/alias_helper.py @@ -74,3 +74,22 @@ def retrieve_import_alias_mapping(names_list): if alias.asname: import_alias_names[alias.asname] = alias.name return import_alias_names + + +def fully_qualify_alias_labels(label, aliases): + """Replace any aliases in label with the fully qualified name. + + Args: + label -- A label : str representing a name (e.g. myos.system) + aliases -- A dict of {alias: real_name} (e.g. {'myos': 'os'}) + + >>> fully_qualify_alias_labels('myos.mycall', {'myos':'os'}) + 'os.mycall' + """ + for alias, full_name in aliases.items(): + if label == alias: + return full_name + if label.startswith(alias+'.'): + return full_name + label[len(alias):] + return label + diff --git a/pyt/cfg/stmt_visitor.py b/pyt/cfg/stmt_visitor.py index 3b9d5f48..e008b096 100644 --- a/pyt/cfg/stmt_visitor.py +++ b/pyt/cfg/stmt_visitor.py @@ -9,7 +9,8 @@ handle_aliases_in_init_files, handle_fdid_aliases, not_as_alias_handler, - retrieve_import_alias_mapping + retrieve_import_alias_mapping, + fully_qualify_alias_labels ) from ..core.ast_helper import ( generate_ast, @@ -61,6 +62,7 @@ class StmtVisitor(ast.NodeVisitor): def __init__(self, allow_local_directory_imports=True): self._allow_local_modules = allow_local_directory_imports + self.bb_or_bi_aliases = {} super().__init__() def visit_Module(self, node): @@ -624,6 +626,10 @@ def add_blackbox_or_builtin_call(self, node, blackbox): # noqa: C901 call_function_label = call_label_visitor.result[:call_label_visitor.result.find('(')] + # Check if function call matches a blackbox/built-in alias and if so, resolve it + # This resolves aliases like "from os import system as mysys" as: mysys -> os.system + call_function_label = fully_qualify_alias_labels(call_function_label, self.bb_or_bi_aliases) + # Create e.g. ~call_1 = ret_func_foo LHS = CALL_IDENTIFIER + 'call_' + str(saved_function_call_index) RHS = 'ret_' + call_function_label + '(' @@ -810,7 +816,6 @@ def add_module( # noqa: C901 module_path = module[1] parent_definitions = self.module_definitions_stack[-1] - # The only place the import_alias_mapping is updated parent_definitions.import_alias_mapping.update(import_alias_mapping) parent_definitions.import_names = local_names @@ -1052,7 +1057,13 @@ def visit_Import(self, node): retrieve_import_alias_mapping(node.names) ) for alias in node.names: - if alias.name not in uninspectable_modules: + if alias.name in uninspectable_modules: + # The module is uninspectable (so blackbox or built-in). If it has an alias, we remember + # the alias so we can do fully qualified name resolution for blackbox- and built-in trigger words + # e.g. we want a call to "os.system" be recognised, even if we do "import os as myos" + if alias.asname is not None and alias.asname != alias.name: + self.bb_or_bi_aliases[alias.asname] = alias.name + else: log.warn("Cannot inspect module %s", alias.name) uninspectable_modules.add(alias.name) # Don't repeatedly warn about this return IgnoredNode() @@ -1094,7 +1105,14 @@ def visit_ImportFrom(self, node): retrieve_import_alias_mapping(node.names), from_from=True ) - if node.module not in uninspectable_modules: + + if node.module in uninspectable_modules: + # Remember aliases for blackboxed and built-in imports such that we can label them fully qualified + # e.g. we want a call to "os.system" be recognised, even if we do "from os import system" + # from os import system as mysystem -> module=os, name=system, asname=mysystem + for name in node.names: + self.bb_or_bi_aliases[name.asname or name.name] = "{}.{}".format(node.module, name.name) + else: log.warn("Cannot inspect module %s", node.module) uninspectable_modules.add(node.module) return IgnoredNode() diff --git a/pyt/vulnerability_definitions/all_trigger_words.pyt b/pyt/vulnerability_definitions/all_trigger_words.pyt index 5642db5c..615e86b6 100644 --- a/pyt/vulnerability_definitions/all_trigger_words.pyt +++ b/pyt/vulnerability_definitions/all_trigger_words.pyt @@ -31,9 +31,10 @@ ] }, "execute(": {}, - "system(": {}, + "os.system(": {}, "filter(": {}, "subprocess.call(": {}, + "subprocess.Popen(": {}, "render_template(": {}, "set_cookie(": {}, "redirect(": {}, @@ -41,7 +42,6 @@ "flash(": {}, "jsonify(": {}, "render(": {}, - "render_to_response(": {}, - "Popen(": {} + "render_to_response(": {} } } diff --git a/tests/main_test.py b/tests/main_test.py index 561d8bd1..fef1e124 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -108,11 +108,11 @@ def test_targets_with_recursive(self): excluded_files = "" included_files = discover_files(targets, excluded_files, True) - self.assertEqual(len(included_files), 33) + self.assertEqual(len(included_files), 34) def test_targets_with_recursive_and_excluded(self): targets = ["examples/vulnerable_code/"] excluded_files = "inter_command_injection.py" included_files = discover_files(targets, excluded_files, True) - self.assertEqual(len(included_files), 32) + self.assertEqual(len(included_files), 33) diff --git a/tests/vulnerabilities/vulnerabilities_test.py b/tests/vulnerabilities/vulnerabilities_test.py index 5a28aa03..fe2fa64c 100644 --- a/tests/vulnerabilities/vulnerabilities_test.py +++ b/tests/vulnerabilities/vulnerabilities_test.py @@ -482,6 +482,21 @@ def test_list_append_taints_list(self): ) self.assert_length(vulnerabilities, expected_length=1) + def test_import_bb_or_bi_with_alias(self): + self.cfg_create_from_file('examples/vulnerable_code/command_injection_with_aliases.py') + + EXPECTED = ['Entry module', + "~call_1 = ret_os.system('ls')", + "~call_2 = ret_os.system('ls')", + "~call_3 = ret_os.system('ls')", + "~call_4 = ret_os.system('ls')", + "~call_5 = ret_subprocess.call('ls')", + "~call_6 = ret_subprocess.Popen('ls')", + 'Exit module' + ] + for node, expected_label in zip(self.cfg.nodes, EXPECTED): + self.assertEqual(node.label, expected_label) + class EngineDjangoTest(VulnerabilitiesBaseTestCase): def run_analysis(self, path): From 57448a0c7761ee7cc054b1630ec1e66e5581f850 Mon Sep 17 00:00:00 2001 From: KevinHock Date: Sat, 23 Mar 2019 12:33:59 -0700 Subject: [PATCH 17/27] :wave: Add `no longer maintained` section --- README.rst | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/README.rst b/README.rst index 025a3705..a84df88d 100644 --- a/README.rst +++ b/README.rst @@ -19,6 +19,26 @@ .. image:: https://img.shields.io/badge/Donate-Charity-orange.svg :target: https://www.againstmalaria.com/donation.aspx +This project is no longer maintained +==================================== + +`Pyre`_ from Facebook is an amazing project that has a bright future and many smart people working on it. +I would suggest, if you don't know that much about program analysis, that you understand how PyT works before diving into Pyre. Along with the `README's in most directories`_, there is the original `Master's Thesis`_ and `some slides`_. +With that said, I am happy to review pull requests and give you write permissions if you make more than a few. + +There were a lot of great contributors to this project, I plan on working on other projects like `detect-secrets`_ and others (e.g. Pyre eventually) in the future if you'd like to work together more :) + +If you are a security engineer with e.g. a Python codebase without type annotations, that Pyre won't handle, I would suggest you replace your sinks with a secure wrapper (something like `defusedxml`_), and alert off any uses of the standard sink. You can use `Bandit`_ to do this but you will have to trim it a lot, due to the high false-positive rate. + +.. _Pyre: https://github.com/facebook/pyre-check +.. _README's in most directories: https://github.com/python-security/pyt/tree/master/pyt#how-it-works +.. _Master's Thesis: https://projekter.aau.dk/projekter/files/239563289/final.pdf +.. _some slides: https://docs.google.com/presentation/d/1JfAykAxR0DcJwwGfHmhrz1RhhKqYsnt5x_GY8CbTp7s +.. _detect-secrets: https://github.com/Yelp/detect-secrets/blob/master/CHANGELOG.md#whats-new +.. _defusedxml: https://pypi.org/project/defusedxml/ +.. _Bandit: https://github.com/PyCQA/bandit + + Python Taint ============ From 21e6027efb94669c6374615350373de850e623a4 Mon Sep 17 00:00:00 2001 From: KevinHock Date: Sat, 23 Mar 2019 12:37:53 -0700 Subject: [PATCH 18/27] Bold important text --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index a84df88d..831a392d 100644 --- a/README.rst +++ b/README.rst @@ -24,7 +24,7 @@ This project is no longer maintained `Pyre`_ from Facebook is an amazing project that has a bright future and many smart people working on it. I would suggest, if you don't know that much about program analysis, that you understand how PyT works before diving into Pyre. Along with the `README's in most directories`_, there is the original `Master's Thesis`_ and `some slides`_. -With that said, I am happy to review pull requests and give you write permissions if you make more than a few. +With that said, **I am happy to review pull requests and give you write permissions if you make more than a few.** There were a lot of great contributors to this project, I plan on working on other projects like `detect-secrets`_ and others (e.g. Pyre eventually) in the future if you'd like to work together more :) From 1ff3901b89c0db437a6b3b613f3c262be10cd61c Mon Sep 17 00:00:00 2001 From: KevinHock Date: Sat, 23 Mar 2019 12:38:55 -0700 Subject: [PATCH 19/27] [grammar] is -> are --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 831a392d..852a8b2d 100644 --- a/README.rst +++ b/README.rst @@ -23,7 +23,7 @@ This project is no longer maintained ==================================== `Pyre`_ from Facebook is an amazing project that has a bright future and many smart people working on it. -I would suggest, if you don't know that much about program analysis, that you understand how PyT works before diving into Pyre. Along with the `README's in most directories`_, there is the original `Master's Thesis`_ and `some slides`_. +I would suggest, if you don't know that much about program analysis, that you understand how PyT works before diving into Pyre. Along with the `README's in most directories`_, there are the original `Master's Thesis`_ and `some slides`_. With that said, **I am happy to review pull requests and give you write permissions if you make more than a few.** There were a lot of great contributors to this project, I plan on working on other projects like `detect-secrets`_ and others (e.g. Pyre eventually) in the future if you'd like to work together more :) From 022476a014ed3a10ff04dd7d2450192e676c9100 Mon Sep 17 00:00:00 2001 From: KevinHock Date: Sat, 23 Mar 2019 12:39:53 -0700 Subject: [PATCH 20/27] Update README.rst --- README.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 852a8b2d..32dba718 100644 --- a/README.rst +++ b/README.rst @@ -28,7 +28,7 @@ With that said, **I am happy to review pull requests and give you write permissi There were a lot of great contributors to this project, I plan on working on other projects like `detect-secrets`_ and others (e.g. Pyre eventually) in the future if you'd like to work together more :) -If you are a security engineer with e.g. a Python codebase without type annotations, that Pyre won't handle, I would suggest you replace your sinks with a secure wrapper (something like `defusedxml`_), and alert off any uses of the standard sink. You can use `Bandit`_ to do this but you will have to trim it a lot, due to the high false-positive rate. +If you are a security engineer with e.g. a Python codebase without type annotations, that Pyre won't handle, I would suggest you replace your sinks with a secure wrapper (something like `defusedxml`_), and alert off any uses of the standard sink. You can use `Bandit`_ to do this since dataflow analysis is not required, but you will have to trim it a lot, due to the high false-positive rate. .. _Pyre: https://github.com/facebook/pyre-check .. _README's in most directories: https://github.com/python-security/pyt/tree/master/pyt#how-it-works From 36bd520cf3e52e218aa79d23cb3113a934551bc6 Mon Sep 17 00:00:00 2001 From: wchresta <34962284+wchresta@users.noreply.github.com> Date: Sat, 23 Mar 2019 16:30:01 -0400 Subject: [PATCH 21/27] Use import_alias_mapping for blackbox and built-in aliases, as well * This will give fully qualified names for blackboxes like flask * Improve readability by using keyword arguments --- .../command_injection_with_aliases.py | 31 +++- pyt/cfg/stmt_visitor.py | 149 +++++++++--------- .../vulnerabilities_across_files_test.py | 2 +- .../vulnerabilities_base_test_case.py | 4 +- tests/vulnerabilities/vulnerabilities_test.py | 58 +++---- 5 files changed, 132 insertions(+), 112 deletions(-) diff --git a/examples/vulnerable_code/command_injection_with_aliases.py b/examples/vulnerable_code/command_injection_with_aliases.py index 309e5268..4e409c52 100644 --- a/examples/vulnerable_code/command_injection_with_aliases.py +++ b/examples/vulnerable_code/command_injection_with_aliases.py @@ -4,9 +4,28 @@ from os import system as mysystem from subprocess import call as mycall, Popen as mypopen -os.system("ls") -myos.system("ls") -system("ls") -mysystem("ls") -mycall("ls") -mypopen("ls") +from flask import Flask, render_template, request + +app = Flask(__name__) + + +@app.route('/menu', methods=['POST']) +def menu(): + param = request.form['suggestion'] + command = 'echo ' + param + ' >> ' + 'menu.txt' + + os.system(command) + myos.system(command) + system(command) + mysystem(command) + mycall(command) + mypopen(command) + + with open('menu.txt', 'r') as f: + menu_ctx = f.read() + + return render_template('command_injection.html', menu=menu_ctx) + + +if __name__ == '__main__': + app.run(debug=True) diff --git a/pyt/cfg/stmt_visitor.py b/pyt/cfg/stmt_visitor.py index e008b096..0a35c566 100644 --- a/pyt/cfg/stmt_visitor.py +++ b/pyt/cfg/stmt_visitor.py @@ -62,7 +62,6 @@ class StmtVisitor(ast.NodeVisitor): def __init__(self, allow_local_directory_imports=True): self._allow_local_modules = allow_local_directory_imports - self.bb_or_bi_aliases = {} super().__init__() def visit_Module(self, node): @@ -628,7 +627,8 @@ def add_blackbox_or_builtin_call(self, node, blackbox): # noqa: C901 # Check if function call matches a blackbox/built-in alias and if so, resolve it # This resolves aliases like "from os import system as mysys" as: mysys -> os.system - call_function_label = fully_qualify_alias_labels(call_function_label, self.bb_or_bi_aliases) + local_definitions = self.module_definitions_stack[-1] + call_function_label = fully_qualify_alias_labels(call_function_label, local_definitions.import_alias_mapping) # Create e.g. ~call_1 = ret_func_foo LHS = CALL_IDENTIFIER + 'call_' + str(saved_function_call_index) @@ -924,10 +924,10 @@ def from_directory_import( if init_exists and not skip_init: package_name = os.path.split(module_path)[1] return self.add_module( - (module[0], init_file_location), - package_name, - local_names, - import_alias_mapping, + module=(module[0], init_file_location), + module_or_package_name=package_name, + local_names=local_names, + import_alias_mapping=import_alias_mapping, is_init=True, from_from=True ) @@ -937,10 +937,10 @@ def from_directory_import( new_init_file_location = os.path.join(full_name, '__init__.py') if os.path.isfile(new_init_file_location): self.add_module( - (real_name, new_init_file_location), - real_name, - local_names, - import_alias_mapping, + module=(real_name, new_init_file_location), + module_or_package_name=real_name, + local_names=local_names, + import_alias_mapping=import_alias_mapping, is_init=True, from_from=True, from_fdid=True @@ -950,10 +950,10 @@ def from_directory_import( else: file_module = (real_name, full_name + '.py') self.add_module( - file_module, - real_name, - local_names, - import_alias_mapping, + module=file_module, + module_or_package_name=real_name, + local_names=local_names, + import_alias_mapping=import_alias_mapping, from_from=True ) return IgnoredNode() @@ -964,10 +964,10 @@ def import_package(self, module, module_name, local_name, import_alias_mapping): init_exists = os.path.isfile(init_file_location) if init_exists: return self.add_module( - (module[0], init_file_location), - module_name, - local_name, - import_alias_mapping, + module=(module[0], init_file_location), + module_or_package_name=module_name, + local_names=local_name, + import_alias_mapping=import_alias_mapping, is_init=True ) else: @@ -1010,10 +1010,10 @@ def handle_relative_import(self, node): # Is it a file? if name_with_dir.endswith('.py'): return self.add_module( - (node.module, name_with_dir), - None, - as_alias_handler(node.names), - retrieve_import_alias_mapping(node.names), + module=(node.module, name_with_dir), + module_or_package_name=None, + local_names=as_alias_handler(node.names), + import_alias_mapping=retrieve_import_alias_mapping(node.names), from_from=True ) return self.from_directory_import( @@ -1036,10 +1036,10 @@ def visit_Import(self, node): retrieve_import_alias_mapping(node.names) ) return self.add_module( - module, - name.name, - name.asname, - retrieve_import_alias_mapping(node.names) + module=module, + module_or_package_name=name.name, + local_names=name.asname, + import_alias_mapping=retrieve_import_alias_mapping(node.names) ) for module in self.project_modules: if name.name == module[0]: @@ -1051,20 +1051,20 @@ def visit_Import(self, node): retrieve_import_alias_mapping(node.names) ) return self.add_module( - module, - name.name, - name.asname, - retrieve_import_alias_mapping(node.names) + module=module, + module_or_package_name=name.name, + local_names=name.asname, + import_alias_mapping=retrieve_import_alias_mapping(node.names) ) for alias in node.names: - if alias.name in uninspectable_modules: - # The module is uninspectable (so blackbox or built-in). If it has an alias, we remember - # the alias so we can do fully qualified name resolution for blackbox- and built-in trigger words - # e.g. we want a call to "os.system" be recognised, even if we do "import os as myos" - if alias.asname is not None and alias.asname != alias.name: - self.bb_or_bi_aliases[alias.asname] = alias.name - else: - log.warn("Cannot inspect module %s", alias.name) + # The module is uninspectable (so blackbox or built-in). If it has an alias, we remember + # the alias so we can do fully qualified name resolution for blackbox- and built-in trigger words + # e.g. we want a call to "os.system" be recognised, even if we do "import os as myos" + if alias.asname is not None and alias.asname != alias.name: + local_definitions = self.module_definitions_stack[-1] + local_definitions.import_alias_mapping[name.asname] = alias.name + if alias.name not in uninspectable_modules: + log.warning("Cannot inspect module %s", alias.name) uninspectable_modules.add(alias.name) # Don't repeatedly warn about this return IgnoredNode() @@ -1072,47 +1072,48 @@ def visit_ImportFrom(self, node): # Is it relative? if node.level > 0: return self.handle_relative_import(node) - else: - for module in self.local_modules: - if node.module == module[0]: - if os.path.isdir(module[1]): - return self.from_directory_import( - module, - not_as_alias_handler(node.names), - as_alias_handler(node.names) - ) - return self.add_module( + # not relative + for module in self.local_modules: + if node.module == module[0]: + if os.path.isdir(module[1]): + return self.from_directory_import( module, - None, - as_alias_handler(node.names), - retrieve_import_alias_mapping(node.names), - from_from=True + not_as_alias_handler(node.names), + as_alias_handler(node.names) ) - for module in self.project_modules: - name = module[0] - if node.module == name: - if os.path.isdir(module[1]): - return self.from_directory_import( - module, - not_as_alias_handler(node.names), - as_alias_handler(node.names), - retrieve_import_alias_mapping(node.names) - ) - return self.add_module( + return self.add_module( + module=module, + module_or_package_name=None, + local_names=as_alias_handler(node.names), + import_alias_mapping=retrieve_import_alias_mapping(node.names), + from_from=True + ) + for module in self.project_modules: + name = module[0] + if node.module == name: + if os.path.isdir(module[1]): + return self.from_directory_import( module, - None, + not_as_alias_handler(node.names), as_alias_handler(node.names), - retrieve_import_alias_mapping(node.names), - from_from=True + retrieve_import_alias_mapping(node.names) ) + return self.add_module( + module=module, + module_or_package_name=None, + local_names=as_alias_handler(node.names), + import_alias_mapping=retrieve_import_alias_mapping(node.names), + from_from=True + ) - if node.module in uninspectable_modules: - # Remember aliases for blackboxed and built-in imports such that we can label them fully qualified - # e.g. we want a call to "os.system" be recognised, even if we do "from os import system" - # from os import system as mysystem -> module=os, name=system, asname=mysystem - for name in node.names: - self.bb_or_bi_aliases[name.asname or name.name] = "{}.{}".format(node.module, name.name) - else: - log.warn("Cannot inspect module %s", node.module) + # Remember aliases for uninspecatble modules such that we can label them fully qualified + # e.g. we want a call to "os.system" be recognised, even if we do "from os import system" + # from os import system as mysystem -> module=os, name=system, asname=mysystem + for name in node.names: + local_definitions = self.module_definitions_stack[-1] + local_definitions.import_alias_mapping[name.asname or name.name] = "{}.{}".format(node.module, name.name) + + if node.module not in uninspectable_modules: + log.warning("Cannot inspect module %s", node.module) uninspectable_modules.add(node.module) return IgnoredNode() diff --git a/tests/vulnerabilities/vulnerabilities_across_files_test.py b/tests/vulnerabilities/vulnerabilities_across_files_test.py index bd63b190..d7b8f0d1 100644 --- a/tests/vulnerabilities/vulnerabilities_across_files_test.py +++ b/tests/vulnerabilities/vulnerabilities_across_files_test.py @@ -62,7 +62,7 @@ def test_blackbox_library_call(self): EXPECTED_VULNERABILITY_DESCRIPTION = """ File: examples/vulnerable_code_across_files/blackbox_library_call.py > User input at line 12, source "request.args.get(": - ~call_1 = ret_request.args.get('suggestion') + ~call_1 = ret_flask.request.args.get('suggestion') Reassigned in: File: examples/vulnerable_code_across_files/blackbox_library_call.py > Line 12: param = ~call_1 diff --git a/tests/vulnerabilities/vulnerabilities_base_test_case.py b/tests/vulnerabilities/vulnerabilities_base_test_case.py index c21f81ed..a7e86121 100644 --- a/tests/vulnerabilities/vulnerabilities_base_test_case.py +++ b/tests/vulnerabilities/vulnerabilities_base_test_case.py @@ -11,7 +11,7 @@ def string_compare_alpha(self, output, expected_string): def assertAlphaEqual(self, output, expected_string): self.assertEqual( - [char for char in output if char.isalpha()], - [char for char in expected_string if char.isalpha()] + ''.join(char for char in output if char.isalpha()), + ''.join(char for char in expected_string if char.isalpha()) ) return True diff --git a/tests/vulnerabilities/vulnerabilities_test.py b/tests/vulnerabilities/vulnerabilities_test.py index fe2fa64c..893ec70a 100644 --- a/tests/vulnerabilities/vulnerabilities_test.py +++ b/tests/vulnerabilities/vulnerabilities_test.py @@ -150,7 +150,7 @@ def test_XSS_result(self): EXPECTED_VULNERABILITY_DESCRIPTION = """ File: examples/vulnerable_code/XSS.py > User input at line 6, source "request.args.get(": - ~call_1 = ret_request.args.get('param', 'not set') + ~call_1 = ret_flask.request.args.get('param', 'not set') Reassigned in: File: examples/vulnerable_code/XSS.py > Line 6: param = ~call_1 @@ -186,7 +186,7 @@ def test_path_traversal_result(self): EXPECTED_VULNERABILITY_DESCRIPTION = """ File: examples/vulnerable_code/path_traversal.py > User input at line 15, source "request.args.get(": - ~call_1 = ret_request.args.get('image_name') + ~call_1 = ret_flask.request.args.get('image_name') Reassigned in: File: examples/vulnerable_code/path_traversal.py > Line 15: image_name = ~call_1 @@ -210,7 +210,7 @@ def test_path_traversal_result(self): > Line 19: foo = ~call_2 File: examples/vulnerable_code/path_traversal.py > reaches line 20, sink "send_file(": - ~call_4 = ret_send_file(foo) + ~call_4 = ret_flask.send_file(foo) """ self.assertAlphaEqual(vulnerability_description, EXPECTED_VULNERABILITY_DESCRIPTION) @@ -222,7 +222,7 @@ def test_ensure_saved_scope(self): EXPECTED_VULNERABILITY_DESCRIPTION = """ File: examples/vulnerable_code/ensure_saved_scope.py > User input at line 15, source "request.args.get(": - ~call_1 = ret_request.args.get('image_name') + ~call_1 = ret_flask.request.args.get('image_name') Reassigned in: File: examples/vulnerable_code/ensure_saved_scope.py > Line 15: image_name = ~call_1 @@ -232,7 +232,7 @@ def test_ensure_saved_scope(self): > Line 10: save_3_image_name = image_name File: examples/vulnerable_code/ensure_saved_scope.py > reaches line 20, sink "send_file(": - ~call_4 = ret_send_file(image_name) + ~call_4 = ret_flask.send_file(image_name) """ self.assertAlphaEqual( vulnerability_description, @@ -246,7 +246,7 @@ def test_path_traversal_sanitised_result(self): EXPECTED_VULNERABILITY_DESCRIPTION = """ File: examples/vulnerable_code/path_traversal_sanitised.py > User input at line 8, source "request.args.get(": - ~call_1 = ret_request.args.get('image_name') + ~call_1 = ret_flask.request.args.get('image_name') Reassigned in: File: examples/vulnerable_code/path_traversal_sanitised.py > Line 8: image_name = ~call_1 @@ -258,7 +258,7 @@ def test_path_traversal_sanitised_result(self): > Line 12: ~call_4 = ret_os.path.join(~call_5, image_name) File: examples/vulnerable_code/path_traversal_sanitised.py > reaches line 12, sink "send_file(": - ~call_3 = ret_send_file(~call_4) + ~call_3 = ret_flask.send_file(~call_4) This vulnerability is sanitised by: Label: ~call_2 = ret_image_name.replace('..', '') """ @@ -271,7 +271,7 @@ def test_path_traversal_sanitised_2_result(self): EXPECTED_VULNERABILITY_DESCRIPTION = """ File: examples/vulnerable_code/path_traversal_sanitised_2.py > User input at line 8, source "request.args.get(": - ~call_1 = ret_request.args.get('image_name') + ~call_1 = ret_flask.request.args.get('image_name') Reassigned in: File: examples/vulnerable_code/path_traversal_sanitised_2.py > Line 8: image_name = ~call_1 @@ -279,7 +279,7 @@ def test_path_traversal_sanitised_2_result(self): > Line 12: ~call_3 = ret_os.path.join(~call_4, image_name) File: examples/vulnerable_code/path_traversal_sanitised_2.py > reaches line 12, sink "send_file(": - ~call_2 = ret_send_file(~call_3) + ~call_2 = ret_flask.send_file(~call_3) This vulnerability is potentially sanitised by: Label: if '..' in image_name: """ @@ -292,7 +292,7 @@ def test_sql_result(self): EXPECTED_VULNERABILITY_DESCRIPTION = """ File: examples/vulnerable_code/sql/sqli.py > User input at line 26, source "request.args.get(": - ~call_1 = ret_request.args.get('param', 'not set') + ~call_1 = ret_flask.request.args.get('param', 'not set') Reassigned in: File: examples/vulnerable_code/sql/sqli.py > Line 26: param = ~call_1 @@ -347,7 +347,7 @@ def test_XSS_reassign_result(self): EXPECTED_VULNERABILITY_DESCRIPTION = """ File: examples/vulnerable_code/XSS_reassign.py > User input at line 6, source "request.args.get(": - ~call_1 = ret_request.args.get('param', 'not set') + ~call_1 = ret_flask.request.args.get('param', 'not set') Reassigned in: File: examples/vulnerable_code/XSS_reassign.py > Line 6: param = ~call_1 @@ -367,18 +367,18 @@ def test_XSS_sanitised_result(self): EXPECTED_VULNERABILITY_DESCRIPTION = """ File: examples/vulnerable_code/XSS_sanitised.py > User input at line 7, source "request.args.get(": - ~call_1 = ret_request.args.get('param', 'not set') + ~call_1 = ret_flask.request.args.get('param', 'not set') Reassigned in: File: examples/vulnerable_code/XSS_sanitised.py > Line 7: param = ~call_1 File: examples/vulnerable_code/XSS_sanitised.py - > Line 9: ~call_2 = ret_Markup.escape(param) + > Line 9: ~call_2 = ret_flask.Markup.escape(param) File: examples/vulnerable_code/XSS_sanitised.py > Line 9: param = ~call_2 File: examples/vulnerable_code/XSS_sanitised.py > reaches line 12, sink "replace(": ~call_5 = ret_html.replace('{{ param }}', param) - This vulnerability is sanitised by: Label: ~call_2 = ret_Markup.escape(param) + This vulnerability is sanitised by: Label: ~call_2 = ret_flask.Markup.escape(param) """ self.assertAlphaEqual(vulnerability_description, EXPECTED_VULNERABILITY_DESCRIPTION) @@ -394,7 +394,7 @@ def test_XSS_variable_assign_result(self): EXPECTED_VULNERABILITY_DESCRIPTION = """ File: examples/vulnerable_code/XSS_variable_assign.py > User input at line 6, source "request.args.get(": - ~call_1 = ret_request.args.get('param', 'not set') + ~call_1 = ret_flask.request.args.get('param', 'not set') Reassigned in: File: examples/vulnerable_code/XSS_variable_assign.py > Line 6: param = ~call_1 @@ -414,7 +414,7 @@ def test_XSS_variable_multiple_assign_result(self): EXPECTED_VULNERABILITY_DESCRIPTION = """ File: examples/vulnerable_code/XSS_variable_multiple_assign.py > User input at line 6, source "request.args.get(": - ~call_1 = ret_request.args.get('param', 'not set') + ~call_1 = ret_flask.request.args.get('param', 'not set') Reassigned in: File: examples/vulnerable_code/XSS_variable_multiple_assign.py > Line 6: param = ~call_1 @@ -483,19 +483,19 @@ def test_list_append_taints_list(self): self.assert_length(vulnerabilities, expected_length=1) def test_import_bb_or_bi_with_alias(self): - self.cfg_create_from_file('examples/vulnerable_code/command_injection_with_aliases.py') - - EXPECTED = ['Entry module', - "~call_1 = ret_os.system('ls')", - "~call_2 = ret_os.system('ls')", - "~call_3 = ret_os.system('ls')", - "~call_4 = ret_os.system('ls')", - "~call_5 = ret_subprocess.call('ls')", - "~call_6 = ret_subprocess.Popen('ls')", - 'Exit module' + vulnerabilities = self.run_analysis('examples/vulnerable_code/command_injection_with_aliases.py') + + EXPECTED_SINK_TRIGGER_WORDS = [ + 'os.system(', + 'os.system(', + 'os.system(', + 'os.system(', + 'subprocess.call(', + 'subprocess.Popen(' ] - for node, expected_label in zip(self.cfg.nodes, EXPECTED): - self.assertEqual(node.label, expected_label) + + for vuln, expected_sink_trigger_word in zip(vulnerabilities, EXPECTED_SINK_TRIGGER_WORDS): + self.assertEqual(vuln.sink_trigger_word, expected_sink_trigger_word) class EngineDjangoTest(VulnerabilitiesBaseTestCase): @@ -531,7 +531,7 @@ def test_django_view_param(self): param File: examples/vulnerable_code/django_XSS.py > reaches line 5, sink "render(": - ~call_1 = ret_render(request, 'templates/xss.html', 'param'param) + ~call_1 = ret_django.shortcuts.render(request, 'templates/xss.html', 'param'param) """ self.assertAlphaEqual(vulnerability_description, EXPECTED_VULNERABILITY_DESCRIPTION) From 61f0408574685f6fb2a1ffd4c39a49964056aa54 Mon Sep 17 00:00:00 2001 From: wchresta <34962284+wchresta@users.noreply.github.com> Date: Sat, 23 Mar 2019 16:45:29 -0400 Subject: [PATCH 22/27] Remove blank line at end of file. --- pyt/cfg/alias_helper.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pyt/cfg/alias_helper.py b/pyt/cfg/alias_helper.py index 9de6d058..9d780992 100644 --- a/pyt/cfg/alias_helper.py +++ b/pyt/cfg/alias_helper.py @@ -92,4 +92,3 @@ def fully_qualify_alias_labels(label, aliases): if label.startswith(alias+'.'): return full_name + label[len(alias):] return label - From a7bb0b275ffc78c4cc168ae529e5adb7467979cd Mon Sep 17 00:00:00 2001 From: wchresta <34962284+wchresta@users.noreply.github.com> Date: Sat, 23 Mar 2019 18:13:50 -0400 Subject: [PATCH 23/27] Implement suggestions from code-review. --- pyt/cfg/alias_helper.py | 2 +- pyt/cfg/stmt_visitor.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/pyt/cfg/alias_helper.py b/pyt/cfg/alias_helper.py index 9d780992..920af3f5 100644 --- a/pyt/cfg/alias_helper.py +++ b/pyt/cfg/alias_helper.py @@ -89,6 +89,6 @@ def fully_qualify_alias_labels(label, aliases): for alias, full_name in aliases.items(): if label == alias: return full_name - if label.startswith(alias+'.'): + elif label.startswith(alias+'.'): return full_name + label[len(alias):] return label diff --git a/pyt/cfg/stmt_visitor.py b/pyt/cfg/stmt_visitor.py index 0a35c566..16da1bb0 100644 --- a/pyt/cfg/stmt_visitor.py +++ b/pyt/cfg/stmt_visitor.py @@ -6,11 +6,11 @@ from .alias_helper import ( as_alias_handler, + fully_qualify_alias_labels, handle_aliases_in_init_files, handle_fdid_aliases, not_as_alias_handler, - retrieve_import_alias_mapping, - fully_qualify_alias_labels + retrieve_import_alias_mapping ) from ..core.ast_helper import ( generate_ast, @@ -816,6 +816,7 @@ def add_module( # noqa: C901 module_path = module[1] parent_definitions = self.module_definitions_stack[-1] + # Here, in `visit_Import` and in `visit_ImportFrom` are the only places the `import_alias_mapping` is updated parent_definitions.import_alias_mapping.update(import_alias_mapping) parent_definitions.import_names = local_names @@ -1106,7 +1107,7 @@ def visit_ImportFrom(self, node): from_from=True ) - # Remember aliases for uninspecatble modules such that we can label them fully qualified + # Remember aliases for uninspectable modules such that we can label them fully qualified # e.g. we want a call to "os.system" be recognised, even if we do "from os import system" # from os import system as mysystem -> module=os, name=system, asname=mysystem for name in node.names: From 9d2a607001e038bda86bf2aa4f0d7fde5751d274 Mon Sep 17 00:00:00 2001 From: wchresta <34962284+wchresta@users.noreply.github.com> Date: Sat, 23 Mar 2019 18:37:20 -0400 Subject: [PATCH 24/27] Add fully qualified shell injection sinks. --- .../all_trigger_words.pyt | 47 +++++++++++++++---- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/pyt/vulnerability_definitions/all_trigger_words.pyt b/pyt/vulnerability_definitions/all_trigger_words.pyt index 615e86b6..edd4649c 100644 --- a/pyt/vulnerability_definitions/all_trigger_words.pyt +++ b/pyt/vulnerability_definitions/all_trigger_words.pyt @@ -30,18 +30,49 @@ "'..' in" ] }, + "commands.getoutput(": {}, + "commands.getstatusoutput(": {}, "execute(": {}, - "os.system(": {}, "filter(": {}, - "subprocess.call(": {}, - "subprocess.Popen(": {}, - "render_template(": {}, - "set_cookie(": {}, - "redirect(": {}, - "url_for(": {}, "flash(": {}, "jsonify(": {}, + "os.execl(": {}, + "os.execle(": {}, + "os.execlp(": {}, + "os.execlpe(": {}, + "os.execv(": {}, + "os.execve(": {}, + "os.execvp(": {}, + "os.execvpe(": {}, + "os.popen(": {}, + "os.popen2(": {}, + "os.popen3(": {}, + "os.popen4(": {}, + "os.spawnl(": {}, + "os.spawnle(": {}, + "os.spawnlp(": {}, + "os.spawnlpe(": {}, + "os.spawnv(": {}, + "os.spawnve(": {}, + "os.spawnvp(": {}, + "os.spawnvpe(": {}, + "os.startfile(": {}, + "os.system(": {}, + "popen2.Popen3(": {}, + "popen2.Popen4(": {}, + "popen2.popen2(": {}, + "popen2.popen3(": {}, + "popen2.popen4(": {}, + "redirect(": {}, "render(": {}, - "render_to_response(": {} + "render_template(": {}, + "render_to_response(": {}, + "set_cookie(": {}, + "subprocess.Popen(": {}, + "subprocess.call(": {}, + "subprocess.check_call(": {}, + "subprocess.check_output(": {}, + "subprocess.run(": {}, + "url_for(": {} } } From e9dc6839d2dd8a126543936cfe6cbca8fdca6beb Mon Sep 17 00:00:00 2001 From: Berkeley Churchill Date: Mon, 12 Aug 2019 21:56:39 -0700 Subject: [PATCH 25/27] Update README.rst Warn users to use python3.6 or 3.7 --- README.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.rst b/README.rst index 32dba718..b0240895 100644 --- a/README.rst +++ b/README.rst @@ -63,6 +63,8 @@ Example usage and output: Install ======= +Before continuing, make sure you have python3.6 or 3.7 installed. + .. code-block:: python pip install python-taint From b51dd7f35e71b1c561afa8d0735390ef1693cf6d Mon Sep 17 00:00:00 2001 From: Berkeley Churchill Date: Tue, 13 Aug 2019 07:04:56 +0000 Subject: [PATCH 26/27] adding eval, exec sinks --- pyt/vulnerability_definitions/all_trigger_words.pyt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pyt/vulnerability_definitions/all_trigger_words.pyt b/pyt/vulnerability_definitions/all_trigger_words.pyt index edd4649c..7d7d2999 100644 --- a/pyt/vulnerability_definitions/all_trigger_words.pyt +++ b/pyt/vulnerability_definitions/all_trigger_words.pyt @@ -32,6 +32,8 @@ }, "commands.getoutput(": {}, "commands.getstatusoutput(": {}, + "eval(": {}, + "exec(": {}, "execute(": {}, "filter(": {}, "flash(": {}, From f4ec9e127497a7ba7d08d68e8fca8b2f06756679 Mon Sep 17 00:00:00 2001 From: KevinHock Date: Sun, 8 Mar 2020 14:43:06 -0700 Subject: [PATCH 27/27] :mortar_board: Add mention of Pysa tutorial https://github.com/facebook/pyre-check/tree/master/pysa_tutorial#pysa-tutorial --- README.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.rst b/README.rst index b0240895..815b430b 100644 --- a/README.rst +++ b/README.rst @@ -22,6 +22,8 @@ This project is no longer maintained ==================================== +**March 2020 Update**: Please go see the amazing `Pysa tutorial`_ that should get you up to speed finding security vulnerabilities in your Python codebase. + `Pyre`_ from Facebook is an amazing project that has a bright future and many smart people working on it. I would suggest, if you don't know that much about program analysis, that you understand how PyT works before diving into Pyre. Along with the `README's in most directories`_, there are the original `Master's Thesis`_ and `some slides`_. With that said, **I am happy to review pull requests and give you write permissions if you make more than a few.** @@ -30,6 +32,7 @@ There were a lot of great contributors to this project, I plan on working on oth If you are a security engineer with e.g. a Python codebase without type annotations, that Pyre won't handle, I would suggest you replace your sinks with a secure wrapper (something like `defusedxml`_), and alert off any uses of the standard sink. You can use `Bandit`_ to do this since dataflow analysis is not required, but you will have to trim it a lot, due to the high false-positive rate. +.. _Pysa tutorial: https://github.com/facebook/pyre-check/tree/master/pysa_tutorial#pysa-tutorial .. _Pyre: https://github.com/facebook/pyre-check .. _README's in most directories: https://github.com/python-security/pyt/tree/master/pyt#how-it-works .. _Master's Thesis: https://projekter.aau.dk/projekter/files/239563289/final.pdf