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 a3b9951

Browse files
Merge pull request #119 from python-security/113_fix_self_false_positive
Skip making self a TaintedNode
2 parents 687626b + 054b706 commit a3b9951

File tree

6 files changed

+70
-19
lines changed

6 files changed

+70
-19
lines changed

‎.pre-commit-config.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@
88
- id: check-ast
99
- id: check-symlinks
1010
- id: flake8
11-
args: ['--ignore=E501']
11+
args: ['--exclude=examples/*', '--ignore=E501,E741']
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
2+
3+
def my_data(self, foo, bar):
4+
return redirect(self.something)

‎pyt/framework_adaptor.py‎

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
from .ast_helper import Arguments
66
from .expr_visitor import make_cfg
77
from .module_definitions import project_definitions
8-
from .node_types import TaintedNode
8+
from .node_types import (
9+
AssignmentNode,
10+
TaintedNode
11+
)
912

1013

1114
class FrameworkAdaptor():
@@ -37,19 +40,27 @@ def get_func_cfg_with_tainted_args(self, definition):
3740
first_node_after_args = func_cfg.nodes[1]
3841
first_node_after_args.ingoing = list()
3942

40-
# We're just gonna give all the tainted args the lineno of the def
43+
# We are just going to give all the tainted args the lineno of the def
4144
definition_lineno = definition.node.lineno
4245

4346
# Taint all the arguments
44-
for arg in args:
45-
tainted_node = TaintedNode(arg, arg,
46-
None, [],
47-
line_number=definition_lineno,
48-
path=definition.path)
49-
function_entry_node.connect(tainted_node)
47+
for i, arg in enumerate(args):
48+
node_type = TaintedNode
49+
if i == 0 and arg == 'self':
50+
node_type = AssignmentNode
51+
52+
arg_node = node_type(
53+
label=arg,
54+
left_hand_side=arg,
55+
ast_node=None,
56+
right_hand_side_variables=[],
57+
line_number=definition_lineno,
58+
path=definition.path
59+
)
60+
function_entry_node.connect(arg_node)
5061
# 1 and not 0 so that Entry Node remains first in the list
51-
func_cfg.nodes.insert(1, tainted_node)
52-
tainted_node.connect(first_node_after_args)
62+
func_cfg.nodes.insert(1, arg_node)
63+
arg_node.connect(first_node_after_args)
5364

5465
return func_cfg
5566

‎pyt/node_types.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,10 @@ def __repr__(self):
176176

177177

178178
class TaintedNode(AssignmentNode):
179+
"""CFG Node that represents a tainted node.
180+
181+
Only created in framework_adaptor.py and only used in `identify_triggers` of vulnerabilities.py
182+
"""
179183
pass
180184

181185

‎tests/vulnerabilities_test.py‎

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,10 @@
1515
from pyt.constraint_table import initialize_constraint_table
1616
from pyt.fixed_point import analyse
1717
from pyt.framework_adaptor import FrameworkAdaptor
18-
from pyt.framework_helper import(
18+
from pyt.framework_helper import(
1919
is_django_view_function,
20-
is_flask_route_function
20+
is_flask_route_function,
21+
is_function
2122
)
2223
from pyt.node_types import Node
2324
from pyt.reaching_definitions_taint import ReachingDefinitionsTaintAnalysis
@@ -95,17 +96,15 @@ def test_find_triggers(self):
9596
l = vulnerabilities.find_triggers(XSS1.nodes, trigger_words)
9697
self.assert_length(l, expected_length=1)
9798

98-
9999
def test_find_sanitiser_nodes(self):
100100
cfg_node = Node(None, None, line_number=None, path=None)
101-
sanitiser_tuple = vulnerabilities.Sanitiser('escape', cfg_node)
101+
sanitiser_tuple = vulnerabilities.Sanitiser('escape', cfg_node)
102102
sanitiser = 'escape'
103103

104104
result = list(vulnerabilities.find_sanitiser_nodes(sanitiser, [sanitiser_tuple]))
105105
self.assert_length(result, expected_length=1)
106106
self.assertEqual(result[0], cfg_node)
107107

108-
109108
def test_build_sanitiser_node_dict(self):
110109
self.cfg_create_from_file('examples/vulnerable_code/XSS_sanitised.py')
111110
cfg_list = [self.cfg]
@@ -114,7 +113,7 @@ def test_build_sanitiser_node_dict(self):
114113

115114
cfg = cfg_list[1]
116115

117-
cfg_node = Node(None, None, line_number=None, path=None)
116+
cfg_node = Node(None, None, line_number=None, path=None)
118117
sinks_in_file = [vulnerabilities.TriggerNode('replace', ['escape'], cfg_node)]
119118

120119
sanitiser_dict = vulnerabilities.build_sanitiser_node_dict(cfg, sinks_in_file)
@@ -142,7 +141,6 @@ def run_analysis(self, path):
142141
)
143142
)
144143

145-
146144
def test_find_vulnerabilities_assign_other_var(self):
147145
vulnerabilities = self.run_analysis('examples/vulnerable_code/XSS_assign_to_other_var.py')
148146
self.assert_length(vulnerabilities, expected_length=1)
@@ -555,3 +553,37 @@ def test_django_view_param(self):
555553
~call_1 = ret_render(request, 'templates/xss.html', 'param'param)
556554
"""
557555
self.assertTrue(self.string_compare_alpha(vulnerability_description, EXPECTED_VULNERABILITY_DESCRIPTION))
556+
557+
558+
class EngineEveryTest(BaseTestCase):
559+
def run_empty(self):
560+
return
561+
562+
def run_analysis(self, path):
563+
self.cfg_create_from_file(path)
564+
cfg_list = [self.cfg]
565+
566+
FrameworkAdaptor(cfg_list, [], [], is_function)
567+
initialize_constraint_table(cfg_list)
568+
569+
analyse(cfg_list, analysis_type=ReachingDefinitionsTaintAnalysis)
570+
571+
trigger_word_file = os.path.join(
572+
'pyt',
573+
'vulnerability_definitions',
574+
'all_trigger_words.pyt'
575+
)
576+
577+
return vulnerabilities.find_vulnerabilities(
578+
cfg_list,
579+
ReachingDefinitionsTaintAnalysis,
580+
UImode.NORMAL,
581+
VulnerabilityFiles(
582+
default_blackbox_mapping_file,
583+
trigger_word_file
584+
)
585+
)
586+
587+
def test_self_is_not_tainted(self):
588+
vulnerabilities = self.run_analysis('examples/example_inputs/def_with_self_as_first_arg.py')
589+
self.assert_length(vulnerabilities, expected_length=0)

‎tox.ini‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ deps = -rrequirements-dev.txt
77
commands =
88
coverage erase
99
coverage run tests
10-
coverage report --show-missing --fail-under 88
10+
coverage report --show-missing --fail-under 89
1111
pre-commit run

0 commit comments

Comments
(0)

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