From 41d666524d2d4a54d25a30883215bc679dd83bf4 Mon Sep 17 00:00:00 2001 From: David O'Callaghan Date: Thu, 30 Nov 2017 15:02:58 +0000 Subject: [PATCH 1/5] Ignore .idea dir --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index c99dd699..fd915640 100644 --- a/.gitignore +++ b/.gitignore @@ -65,3 +65,6 @@ target/ .ipynb_checkpoints *~ *# + +#IDE +.idea/ From cab906beb68fb67213c989575dee6dcff7467f26 Mon Sep 17 00:00:00 2001 From: David O'Callaghan Date: Thu, 30 Nov 2017 15:03:52 +0000 Subject: [PATCH 2/5] Basic Django framework helper --- pyt/__main__.py | 8 +++++--- pyt/framework_helper.py | 10 +++++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/pyt/__main__.py b/pyt/__main__.py index fa37a36f..722ea0ee 100644 --- a/pyt/__main__.py +++ b/pyt/__main__.py @@ -15,8 +15,8 @@ from .framework_helper import ( is_flask_route_function, is_function, - is_function_without_leading_ -) + is_function_without_leading_, + is_django_view_function) from .github_search import scan_github, set_github_api_token from .interprocedural_cfg import interprocedural from .intraprocedural_cfg import intraprocedural @@ -83,7 +83,7 @@ def parse_args(args): help='Choose logging level: CRITICAL, ERROR,' + ' WARNING(Default), INFO, DEBUG, NOTSET.', type=str) parser.add_argument('-a', '--adaptor', - help='Choose an adaptor: Flask(Default) or Every or Pylons.', + help='Choose an adaptor: Flask(Default) or Every or Pylons or Django.', type=str) parser.add_argument('-db', '--create-database', help='Creates a sql file that can be used to' + @@ -226,6 +226,8 @@ def main(command_line_args=sys.argv[1:]): framework_route_criteria = is_function elif args.adaptor and args.adaptor.lower().startswith('p'): framework_route_criteria = is_function_without_leading_ + elif args.adaptor and args.adaptor.lower().startswith('d'): + framework_route_criteria = is_django_view_function else: framework_route_criteria = is_flask_route_function # Add all the route functions to the cfg_list diff --git a/pyt/framework_helper.py b/pyt/framework_helper.py index c35470b4..7e6848fd 100644 --- a/pyt/framework_helper.py +++ b/pyt/framework_helper.py @@ -1,7 +1,8 @@ """Provides helper functions that help with determining if a function is a route function.""" import ast -from .ast_helper import get_call_names +from pyt.base_cfg import Function +from .ast_helper import get_call_names, Arguments def is_function(function): @@ -18,6 +19,13 @@ def is_flask_route_function(ast_node): return False +def is_django_view_function(ast_node): + arguments = Arguments(ast_node.args) + if 'request' in arguments: + return True + return False + + def is_function_without_leading_(ast_node): if ast_node.name.startswith('_'): return False From 88265fbbba103e0f9985353ffa5b55a94a19e0bb Mon Sep 17 00:00:00 2001 From: David O'Callaghan Date: Sat, 2 Dec 2017 15:27:14 +0000 Subject: [PATCH 3/5] Add example Django class-based view and tests --- example/example_inputs/django_views.py | 17 ++++++++ .../flask_function_and_normal_functions.py | 3 ++ pyt/framework_helper.py | 6 +-- tests/framework_helper_test.py | 39 ++++++++++++++++--- 4 files changed, 57 insertions(+), 8 deletions(-) create mode 100644 example/example_inputs/django_views.py diff --git a/example/example_inputs/django_views.py b/example/example_inputs/django_views.py new file mode 100644 index 00000000..8bb17e0c --- /dev/null +++ b/example/example_inputs/django_views.py @@ -0,0 +1,17 @@ +def django_view_function(request, x): + return x + + +class DjangoViewClass(object): + def __init__(self): + pass + + @classmethod + def as_view(cls): + def view_function(request, x): + return x + return view_function + + +# in practice, this would be called in a Django URLconf file +view = DjangoViewClass.as_view() diff --git a/example/example_inputs/flask_function_and_normal_functions.py b/example/example_inputs/flask_function_and_normal_functions.py index bdbb783e..c816b56b 100644 --- a/example/example_inputs/flask_function_and_normal_functions.py +++ b/example/example_inputs/flask_function_and_normal_functions.py @@ -8,4 +8,7 @@ def _hidden_foo(): def flask_function(x): return x +def django_function(request, x): + return x + print('nothing') diff --git a/pyt/framework_helper.py b/pyt/framework_helper.py index 7e6848fd..29c7cccc 100644 --- a/pyt/framework_helper.py +++ b/pyt/framework_helper.py @@ -20,9 +20,9 @@ def is_flask_route_function(ast_node): def is_django_view_function(ast_node): - arguments = Arguments(ast_node.args) - if 'request' in arguments: - return True + if len(ast_node.args.args): + first_arg_name = ast_node.args.args[0].arg + return first_arg_name == 'request' return False diff --git a/tests/framework_helper_test.py b/tests/framework_helper_test.py index 86d2bb1d..f18d2985 100644 --- a/tests/framework_helper_test.py +++ b/tests/framework_helper_test.py @@ -3,7 +3,8 @@ from pyt.framework_helper import ( is_flask_route_function, is_function, - is_function_without_leading_ + is_function_without_leading_, + is_django_view_function ) class FrameworkEngineTest(BaseTestCase): @@ -32,8 +33,8 @@ def test_find_every_function_without_leading_underscore(self): for func in funcs: if is_function_without_leading_(func.node): i = i + 1 - # So it is supposed to be 2, because we count all functions without a leading underscore - self.assertEqual(i, 2) + # So it is supposed to be 3, because we count all functions without a leading underscore + self.assertEqual(i, 3) def test_find_every_function(self): self.cfg_create_from_file('example/example_inputs/flask_function_and_normal_functions.py') @@ -45,5 +46,33 @@ def test_find_every_function(self): for func in funcs: if is_function(func.node): i = i + 1 - # So it is supposed to be 3, because we count all functions - self.assertEqual(len(funcs), 3) + # So it is supposed to be 4, because we count all functions + self.assertEqual(len(funcs), 4) + + def test_find_django_functions(self): + self.cfg_create_from_file('example/example_inputs/flask_function_and_normal_functions.py') + + cfg_list = [self.cfg] + funcs = _get_func_nodes() + + i = 0 + for func in funcs: + if is_django_view_function(func.node): + self.assertEqual(func.node.name, 'django_function') + i = i + 1 + # So it is supposed to be 1 + self.assertEqual(i, 1) + + def test_find_django_views(self): + self.cfg_create_from_file('example/example_inputs/django_views.py') + + cfg_list = [self.cfg] + funcs = _get_func_nodes() + + i = 0 + for func in funcs: + if is_django_view_function(func.node): + self.assertIn('view_function', func.node.name) + i = i + 1 + # So it is supposed to be 2 + self.assertEqual(i, 2) From efbdbd745dc907ed15e713c57129a1ff1b24cb26 Mon Sep 17 00:00:00 2001 From: David O'Callaghan Date: Tue, 12 Dec 2017 21:50:07 +0000 Subject: [PATCH 4/5] Connect all arg tainted nodes to following nodes. Fixes #73 --- pyt/framework_adaptor.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pyt/framework_adaptor.py b/pyt/framework_adaptor.py index 7e00ddfe..c56dfe50 100644 --- a/pyt/framework_adaptor.py +++ b/pyt/framework_adaptor.py @@ -47,9 +47,7 @@ def get_func_cfg_with_tainted_args(self, definition): function_entry_node.connect(tainted_node) # 1 and not 0 so that Entry Node remains first in the list func_cfg.nodes.insert(1, tainted_node) - - first_arg = func_cfg.nodes[len(args)] - first_arg.connect(first_node_after_args) + tainted_node.connect(first_node_after_args) return func_cfg From 738a0f817e890e012fccfad14c0b413d32395bc7 Mon Sep 17 00:00:00 2001 From: David O'Callaghan Date: Tue, 12 Dec 2017 22:09:28 +0000 Subject: [PATCH 5/5] Added Django trigger words and tests - Reworded description of framework function parameter --- example/vulnerable_code/django_XSS.py | 6 +++ .../django_trigger_words.pyt | 32 +++++++++++++++ pyt/vulnerabilities.py | 2 +- tests/vulnerabilities_test.py | 40 ++++++++++++++++++- 4 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 example/vulnerable_code/django_XSS.py create mode 100644 pyt/trigger_definitions/django_trigger_words.pyt diff --git a/example/vulnerable_code/django_XSS.py b/example/vulnerable_code/django_XSS.py new file mode 100644 index 00000000..6dd9560b --- /dev/null +++ b/example/vulnerable_code/django_XSS.py @@ -0,0 +1,6 @@ +from django.shortcuts import render + + +def xss1(request, param): + return render(request, 'templates/xss.html', {'param': param}) + diff --git a/pyt/trigger_definitions/django_trigger_words.pyt b/pyt/trigger_definitions/django_trigger_words.pyt new file mode 100644 index 00000000..53b54f66 --- /dev/null +++ b/pyt/trigger_definitions/django_trigger_words.pyt @@ -0,0 +1,32 @@ +sources: +POST.get( +GET.get( +META.get( +POST[ +GET[ +META[ +FILES[ +.data +form[ +form( +mark_safe( +cookies[ +files[ +SQLAlchemy + +sinks: +replace( -> escape +send_file( -> '..', '..' in +execute( +system( +filter( +subprocess.call( +render_template( +set_cookie( +redirect( +url_for( +flash( +jsonify( +render( +render_to_response( +Popen( \ No newline at end of file diff --git a/pyt/vulnerabilities.py b/pyt/vulnerabilities.py index a180df5b..c9edcf94 100644 --- a/pyt/vulnerabilities.py +++ b/pyt/vulnerabilities.py @@ -68,7 +68,7 @@ def identify_triggers(cfg, sources, sinks, lattice): """ assignment_nodes = filter_cfg_nodes(cfg, AssignmentNode) tainted_nodes = filter_cfg_nodes(cfg, TaintedNode) - tainted_trigger_nodes = [TriggerNode('Flask function URL parameter', None, + tainted_trigger_nodes = [TriggerNode('Framework function URL parameter', None, node) for node in tainted_nodes] sources_in_file = find_triggers(assignment_nodes, sources) sources_in_file.extend(tainted_trigger_nodes) diff --git a/tests/vulnerabilities_test.py b/tests/vulnerabilities_test.py index ccdd05c2..491a382b 100644 --- a/tests/vulnerabilities_test.py +++ b/tests/vulnerabilities_test.py @@ -6,7 +6,7 @@ from pyt.constraint_table import constraint_table, initialize_constraint_table from pyt.fixed_point import analyse from pyt.framework_adaptor import FrameworkAdaptor -from pyt.framework_helper import is_flask_route_function +from pyt.framework_helper import is_flask_route_function, is_django_view_function from pyt.lattice import Lattice from pyt.reaching_definitions_taint import ReachingDefinitionsTaintAnalysis @@ -318,7 +318,7 @@ def test_XSS_url_result(self): vulnerability_description = str(vulnerability_log.vulnerabilities[0]) EXPECTED_VULNERABILITY_DESCRIPTION = """ File: example/vulnerable_code/XSS_url.py - > User input at line 4, trigger word "Flask function URL parameter": + > User input at line 4, trigger word "Framework function URL parameter": url Reassigned in: File: example/vulnerable_code/XSS_url.py @@ -454,3 +454,39 @@ def test_XSS_variable_multiple_assign_result(self): """ self.assertTrue(self.string_compare_alpha(vulnerability_description, EXPECTED_VULNERABILITY_DESCRIPTION)) + + +class EngineDjangoTest(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_django_view_function) + initialize_constraint_table(cfg_list) + + analyse(cfg_list, analysis_type=ReachingDefinitionsTaintAnalysis) + + trigger_word_file = os.path.join('pyt', 'trigger_definitions', 'django_trigger_words.pyt') + + return vulnerabilities.find_vulnerabilities(cfg_list, ReachingDefinitionsTaintAnalysis, trigger_word_file=trigger_word_file) + + def test_django_view_param(self): + vulnerability_log = self.run_analysis('example/vulnerable_code/django_XSS.py') + self.assert_length(vulnerability_log.vulnerabilities, expected_length=2) + vulnerability_description = str(vulnerability_log.vulnerabilities[0]) + + EXPECTED_VULNERABILITY_DESCRIPTION = """ + File: example/vulnerable_code/django_XSS.py + > User input at line 4, trigger word "Framework function URL parameter": + param + Reassigned in: + File: example/vulnerable_code/django_XSS.py + > Line 5: ret_xss1 = ¤call_1 + File: example/vulnerable_code/django_XSS.py + > reaches line 5, trigger word "render(": + ¤call_1 = ret_render(request, 'templates/xss.html', 'param'param) + """ + self.assertTrue(self.string_compare_alpha(vulnerability_description, EXPECTED_VULNERABILITY_DESCRIPTION))