Skip to content

Commit 79d39d8

Browse files
authored
Merge pull request #181 from bcaller/appendaddextendinsert
list is tainted by calling list.append(TAINT)
2 parents 0932cc9 + 7847d01 commit 79d39d8

File tree

5 files changed

+56
-5
lines changed

5 files changed

+56
-5
lines changed
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import os
2+
3+
from flask import request
4+
5+
6+
def func():
7+
TAINT = request.args.get("TAINT")
8+
9+
cmd = []
10+
cmd.append("echo")
11+
cmd.append(TAINT)
12+
13+
os.system(" ".join(cmd))

pyt/cfg/expr_visitor.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
)
2222
from .expr_visitor_helper import (
2323
BUILTINS,
24+
MUTATORS,
2425
return_connection_handler,
2526
SavedVariable
2627
)
@@ -59,6 +60,7 @@ def __init__(
5960
self.module_definitions_stack = list()
6061
self.prev_nodes_to_avoid = list()
6162
self.last_control_flow_nodes = list()
63+
self._within_mutating_call = False
6264

6365
# Are we already in a module?
6466
if module_definitions:
@@ -578,6 +580,23 @@ def visit_Call(self, node):
578580
else:
579581
raise Exception('Definition was neither FunctionDef or ' +
580582
'ClassDef, cannot add the function ')
583+
elif (
584+
not self._within_mutating_call and
585+
last_attribute in MUTATORS
586+
and isinstance(node.func, ast.Attribute)
587+
):
588+
# Change list.append(x) ---> list += list.append(x)
589+
# This does in fact propagate as we don't know that append returns None
590+
fake_aug_assign = ast.AugAssign(
591+
target=node.func.value,
592+
op=ast.Add,
593+
value=node,
594+
)
595+
ast.copy_location(fake_aug_assign, node)
596+
self._within_mutating_call = True # Don't do this recursively
597+
result = self.visit(fake_aug_assign)
598+
self._within_mutating_call = False
599+
return result
581600
elif last_attribute not in BUILTINS:
582601
# Mark the call as a blackbox because we don't have the definition
583602
return self.add_blackbox_or_builtin_call(node, blackbox=True)

pyt/cfg/expr_visitor_helper.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@
3131
'flash',
3232
'jsonify'
3333
)
34+
MUTATORS = ( # list.append(x) taints list if x is tainted
35+
'add',
36+
'append',
37+
'extend',
38+
'insert',
39+
'update',
40+
)
3441

3542

3643
def return_connection_handler(nodes, exit_node):

tests/main_test.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,11 +108,11 @@ def test_targets_with_recursive(self):
108108
excluded_files = ""
109109

110110
included_files = discover_files(targets, excluded_files, True)
111-
self.assertEqual(len(included_files), 32)
111+
self.assertEqual(len(included_files), 33)
112112

113113
def test_targets_with_recursive_and_excluded(self):
114114
targets = ["examples/vulnerable_code/"]
115115
excluded_files = "inter_command_injection.py"
116116

117117
included_files = discover_files(targets, excluded_files, True)
118-
self.assertEqual(len(included_files), 31)
118+
self.assertEqual(len(included_files), 32)

tests/vulnerabilities/vulnerabilities_test.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,20 +110,25 @@ def test_build_sanitiser_node_dict(self):
110110

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

113-
def run_analysis(self, path=None):
113+
def run_analysis(
114+
self,
115+
path=None,
116+
adaptor_function=is_flask_route_function,
117+
trigger_file=default_trigger_word_file,
118+
):
114119
if path:
115120
self.cfg_create_from_file(path)
116121
cfg_list = [self.cfg]
117122

118-
FrameworkAdaptor(cfg_list, [], [], is_flask_route_function)
123+
FrameworkAdaptor(cfg_list, [], [], adaptor_function)
119124
initialize_constraint_table(cfg_list)
120125

121126
analyse(cfg_list)
122127

123128
return find_vulnerabilities(
124129
cfg_list,
125130
default_blackbox_mapping_file,
126-
default_trigger_word_file
131+
trigger_file,
127132
)
128133

129134
def test_find_vulnerabilities_assign_other_var(self):
@@ -470,6 +475,13 @@ def test_recursion(self):
470475
vulnerabilities = self.run_analysis('examples/vulnerable_code/recursive.py')
471476
self.assert_length(vulnerabilities, expected_length=2)
472477

478+
def test_list_append_taints_list(self):
479+
vulnerabilities = self.run_analysis(
480+
'examples/vulnerable_code/list_append.py',
481+
adaptor_function=is_function,
482+
)
483+
self.assert_length(vulnerabilities, expected_length=1)
484+
473485

474486
class EngineDjangoTest(VulnerabilitiesBaseTestCase):
475487
def run_analysis(self, path):

0 commit comments

Comments
 (0)