Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit d23ab96

Browse files
bcallerBen Caller
authored and
Ben Caller
committed
Taint propagates from methods of tainted objects
Previously `x = TAINT.lower()` would be tainted (due to special handling for assignment_call_nodes) but `x = str(TAINT.lower())` wouldn't be tainted. To fix this, `TAINT` is added to the RHS variables of `TAINT.lower()`. This will mean that e.g. `request` will be a RHS variable of `request.get()`, but I think that will be OK. In the test which changed, the additional line is because resp has become tainted. However, this still leaves the following false negatives to fix another day: `assert_vulnerable('result = str("%s" % str(TAINT.lower()))') # FAILS` `assert_vulnerable('result = str("%s" % TAINT.lower().upper())') # FAILS`
1 parent c0e6ace commit d23ab96

File tree

2 files changed

+30
-16
lines changed

2 files changed

+30
-16
lines changed

‎pyt/cfg/stmt_visitor.py

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
)
1212
from ..core.ast_helper import (
1313
generate_ast,
14+
get_call_names,
1415
get_call_names_as_string
1516
)
1617
from ..core.module_definitions import (
@@ -472,14 +473,6 @@ def assignment_call_node(self, left_hand_label, ast_node):
472473
call = self.visit(ast_node.value)
473474
call_label = call.left_hand_side
474475

475-
if isinstance(call, BBorBInode):
476-
# Necessary to know e.g.
477-
# `image_name = image_name.replace('..', '')`
478-
# is a reassignment.
479-
vars_visitor = VarsVisitor()
480-
vars_visitor.visit(ast_node.value)
481-
call.right_hand_side_variables.extend(vars_visitor.result)
482-
483476
call_assignment = AssignmentCallNode(
484477
left_hand_label + ' = ' + call_label,
485478
left_hand_label,
@@ -597,14 +590,14 @@ def add_blackbox_or_builtin_call(self, node, blackbox):
597590
saved_function_call_index = self.function_call_index
598591
self.undecided = False
599592

600-
call_label = LabelVisitor()
601-
call_label.visit(node)
593+
call_label_visitor = LabelVisitor()
594+
call_label_visitor.visit(node)
602595

603-
index = call_label.result.find('(')
596+
call_function_label = call_label_visitor.result[:call_label_visitor.result.find('(')]
604597

605598
# Create e.g. ~call_1 = ret_func_foo
606599
LHS = CALL_IDENTIFIER + 'call_' + str(saved_function_call_index)
607-
RHS = 'ret_' + call_label.result[:index] + '('
600+
RHS = 'ret_' + call_function_label + '('
608601

609602
call_node = BBorBInode(
610603
label='',
@@ -613,7 +606,7 @@ def add_blackbox_or_builtin_call(self, node, blackbox):
613606
right_hand_side_variables=[],
614607
line_number=node.lineno,
615608
path=self.filenames[-1],
616-
func_name=call_label.result[:index]
609+
func_name=call_function_label
617610
)
618611
visual_args = list()
619612
rhs_vars = list()
@@ -657,6 +650,11 @@ def add_blackbox_or_builtin_call(self, node, blackbox):
657650
# `scrypt.outer(scrypt.inner(image_name), scrypt.other_inner(image_name))`
658651
last_return_value_of_nested_call.connect(call_node)
659652

653+
call_names = list(get_call_names(node.func))
654+
if len(call_names) > 1:
655+
# taint is a RHS variable (self) of taint.lower()
656+
rhs_vars.append(call_names[0])
657+
660658
if len(visual_args) > 0:
661659
for arg in visual_args:
662660
RHS = RHS + arg + ", "
@@ -667,7 +665,6 @@ def add_blackbox_or_builtin_call(self, node, blackbox):
667665
call_node.label = LHS + " = " + RHS
668666

669667
call_node.right_hand_side_variables = rhs_vars
670-
# Used in get_sink_args, not using right_hand_side_variables because it is extended in assignment_call_node
671668
rhs_visitor = RHSVisitor()
672669
rhs_visitor.visit(node)
673670
call_node.args = rhs_visitor.result

‎tests/vulnerabilities/vulnerabilities_test.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,9 @@ def test_build_sanitiser_node_dict(self):
111111

112112
self.assertEqual(sanitiser_dict['escape'][0], cfg.nodes[3])
113113

114-
def run_analysis(self, path):
115-
self.cfg_create_from_file(path)
114+
def run_analysis(self, path=None):
115+
if path:
116+
self.cfg_create_from_file(path)
116117
cfg_list = [self.cfg]
117118

118119
FrameworkAdaptor(cfg_list, [], [], is_flask_route_function)
@@ -340,6 +341,8 @@ def test_XSS_form_result(self):
340341
> Line 15: ~call_1 = ret_make_response(~call_2)
341342
File: examples/vulnerable_code/XSS_form.py
342343
> Line 15: resp = ~call_1
344+
File: examples/vulnerable_code/XSS_form.py
345+
> Line 16: ~call_3 = ret_resp.set_cookie('session_id', ~call_4)
343346
File: examples/vulnerable_code/XSS_form.py
344347
> Line 17: ret_example2_action = resp
345348
File: examples/vulnerable_code/XSS_form.py
@@ -517,6 +520,20 @@ def test_yield(self):
517520

518521
self.assertAlphaEqual(str(vuln), EXPECTED_VULNERABILITY_DESCRIPTION)
519522

523+
def test_method_of_taint(self):
524+
def assert_vulnerable(fixture):
525+
tree = ast.parse('TAINT = request.args.get("")\n' + fixture + '\nexecute(result)')
526+
self.cfg_create_from_ast(tree)
527+
vulnerabilities = self.run_analysis()
528+
self.assert_length(vulnerabilities, expected_length=1, msg=fixture)
529+
530+
assert_vulnerable('result = TAINT')
531+
assert_vulnerable('result = TAINT.lower()')
532+
assert_vulnerable('result = str(TAINT)')
533+
assert_vulnerable('result = str(TAINT.lower())')
534+
assert_vulnerable('result = repr(str("%s" % TAINT.lower().upper()))')
535+
assert_vulnerable('result = repr(str("{}".format(TAINT.lower())))')
536+
520537

521538
class EngineDjangoTest(VulnerabilitiesBaseTestCase):
522539
def run_analysis(self, path):

0 commit comments

Comments
(0)

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