Skip to content

Commit aeae217

Browse files
committed
Implement W8012
1 parent 5780d79 commit aeae217

File tree

4 files changed

+228
-11
lines changed

4 files changed

+228
-11
lines changed

pylint_secure_coding_standard.py

Lines changed: 54 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,42 @@ def _is_builtin_open_for_writing(node):
9595
return False
9696

9797

98+
def _is_os_open(node):
99+
return (
100+
isinstance(node.func, astroid.Attribute)
101+
and isinstance(node.func.expr, astroid.Name)
102+
and node.func.attrname == 'open'
103+
and node.func.expr.name == 'os'
104+
)
105+
106+
107+
def _is_os_open_allowed_mode(node, allowed_modes):
108+
if _is_os_open(node):
109+
mode = None
110+
flags = None # pylint: disable=unused-variable
111+
if len(node.args) > 1 and isinstance(node.args[1], (astroid.Attribute, astroid.BinOp)):
112+
# Cover:
113+
# * os.open(xxx, os.O_WRONLY)
114+
# * os.open(xxx, os.O_WRONLY | os.O_CREATE)
115+
# * os.open(xxx, os.O_WRONLY | os.O_CREATE | os.O_FSYNC)
116+
flags = node.args[1]
117+
if len(node.args) > 2 and isinstance(node.args[2], astroid.Const):
118+
mode = node.args[2].value
119+
elif node.keywords:
120+
for keyword in node.keywords:
121+
if keyword.arg == 'flags':
122+
flags = keyword.value # pylint: disable=unused-variable # noqa: F841
123+
if keyword.arg == 'mode':
124+
mode = keyword.value.value
125+
break
126+
if mode is not None:
127+
# TODO: condition check on the flags value if present (ie. ignore if read-only)
128+
return mode in allowed_modes
129+
130+
# NB: default to True in all other cases
131+
return True
132+
133+
98134
def _is_shell_true_call(node):
99135
if not (isinstance(node.func, astroid.Attribute) and isinstance(node.func.expr, astroid.Name)):
100136
return False
@@ -320,7 +356,7 @@ def __init__(self, *args, **kwargs):
320356
"""Initialize a SecureCodingStandardChecker object."""
321357
super().__init__(*args, **kwargs)
322358
self._prefer_os_open = False
323-
self._os_open_mode_allowed = []
359+
self._os_open_modes_allowed = []
324360

325361
def visit_call(self, node):
326362
"""Visitor method called for astroid.Call nodes."""
@@ -346,6 +382,12 @@ def visit_call(self, node):
346382
self.add_message('avoid-eval-exec', node=node)
347383
elif not _is_posix() and _is_shlex_quote_call(node):
348384
self.add_message('avoid-shlex-quote-on-non-posix', node=node)
385+
elif (
386+
_is_os_open(node)
387+
and self._prefer_os_open
388+
and not _is_os_open_allowed_mode(node, self._os_open_modes_allowed)
389+
):
390+
self.add_message('os-open-unsafe-permissions', node=node)
349391

350392
def visit_import(self, node):
351393
"""Visitor method called for astroid.Import nodes."""
@@ -380,8 +422,11 @@ def visit_with(self, node):
380422
"""Visitor method called for astroid.With nodes."""
381423
if self._prefer_os_open:
382424
for item in node.items:
383-
if item and isinstance(item[0], astroid.Call) and _is_builtin_open_for_writing(item[0]):
384-
self.add_message('replace-builtin-open', node=node)
425+
if item and isinstance(item[0], astroid.Call):
426+
if _is_builtin_open_for_writing(item[0]):
427+
self.add_message('replace-builtin-open', node=node)
428+
elif _is_os_open(item[0]) and not _is_os_open_allowed_mode(item[0], self._os_open_modes_allowed):
429+
self.add_message('os-open-unsafe-permissions', node=node)
385430

386431
def visit_assert(self, node):
387432
"""Visitor method called for astroid.Assert nodes."""
@@ -417,8 +462,8 @@ def _update_display_msg(suffix=''):
417462
if len(modes) > 1:
418463
# Lists of allowed modes
419464
try:
420-
self._os_open_mode_allowed = [_str_to_int(mode) for mode in modes if mode]
421-
if not self._os_open_mode_allowed:
465+
self._os_open_modes_allowed = [_str_to_int(mode) for mode in modes if mode]
466+
if not self._os_open_modes_allowed:
422467
raise ValueError('Calculated empty value for `os_open_mode`!')
423468
self._prefer_os_open = True
424469
_update_display_msg(suffix=f' (mode in {modes})')
@@ -430,18 +475,18 @@ def _update_display_msg(suffix=''):
430475
val = _str_to_int(arg)
431476
self._prefer_os_open = val > 0
432477
if self._prefer_os_open:
433-
self._os_open_mode_allowed = list(range(0, val + 1))
478+
self._os_open_modes_allowed = list(range(0, val + 1))
434479
_update_display_msg(suffix=f' (mode <= {arg})')
435480
else:
436-
self._os_open_mode_allowed.clear()
481+
self._os_open_modes_allowed.clear()
437482
except ValueError as error:
438483
if arg in ('y', 'yes', 'true'):
439484
self._prefer_os_open = True
440-
self._os_open_mode_allowed = list(range(0, self.DEFAULT_MAX_MODE + 1))
485+
self._os_open_modes_allowed = list(range(0, self.DEFAULT_MAX_MODE + 1))
441486
_update_display_msg(suffix=f' (mode <= {oct(self.DEFAULT_MAX_MODE)})')
442487
elif arg in ('n', 'no', 'false'):
443488
self._prefer_os_open = False
444-
self._os_open_mode_allowed.clear()
489+
self._os_open_modes_allowed.clear()
445490
else:
446491
raise ValueError(f'Invalid value for `os_open_mode`: {arg}!') from error
447492
else:

pyproject.toml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ build-backend = "setuptools.build_meta"
6161
[tool.pylint.reports]
6262
msg-template = '{path}:{line}: [{msg_id}, {obj}] {msg} ({symbol})'
6363

64+
[tool.pylint.messages_control]
65+
disable = [
66+
'fixme'
67+
]
6468

6569
[tool.pytest.ini_options]
6670
minversion = '6.0'

tests/os_open_test.py

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
# -*- coding: utf-8 -*-
2+
# Copyright 2021 Damien Nguyen
3+
#
4+
# Licensed under the Apache License, Version 2.0 (the "License");
5+
# you may not use this file except in compliance with the License.
6+
# You may obtain a copy of the License at
7+
#
8+
# http://www.apache.org/licenses/LICENSE-2.0
9+
#
10+
# Unless required by applicable law or agreed to in writing, software
11+
# distributed under the License is distributed on an "AS IS" BASIS,
12+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
# See the License for the specific language governing permissions and
14+
# limitations under the License.
15+
16+
import astroid
17+
import pylint.testutils
18+
import pytest
19+
20+
import pylint_secure_coding_standard as pylint_scs
21+
22+
23+
class TestSecureCodingStandardChecker(pylint.testutils.CheckerTestCase):
24+
CHECKER_CLASS = pylint_scs.SecureCodingStandardChecker
25+
26+
@pytest.mark.parametrize(
27+
'arg',
28+
('True', '0o644', '0o644,'),
29+
)
30+
def test_os_open_ok(self, arg):
31+
nodes = astroid.extract_node(
32+
"""
33+
int(0) #@
34+
foo() #@
35+
os.open("file.txt") #@
36+
os.open("file.txt", os.O_RDONLY) #@
37+
os.open("file.txt", os.O_RDONLY, 0o644) #@
38+
os.open("file.txt", os.O_RDONLY | os.O_NOFOLLOW) #@
39+
os.open("file.txt", os.O_RDONLY | os.O_NOFOLLOW, 0o644) #@
40+
bla.open("file.txt") #@
41+
bla.open("file.txt", os.O_RDONLY) #@
42+
bla.open("file.txt", os.O_RDONLY, 0o644) #@
43+
bla.open("file.txt", os.O_RDONLY, 0o755) #@
44+
bla.open("file.txt", os.O_RDONLY | os.O_NOFOLLOW) #@
45+
bla.open("file.txt", os.O_RDONLY | os.O_NOFOLLOW, 0o644) #@
46+
bla.open("file.txt", os.O_RDONLY | os.O_NOFOLLOW, 0o755) #@
47+
with os.open("file.txt") as fd: fd.read() #@
48+
with os.open("file.txt", os.O_RDONLY) as fd: fd.read() #@
49+
with os.open("file.txt", os.O_RDONLY, 0o644) as fd: fd.read() #@
50+
with os.open("file.txt", os.O_RDONLY | os.O_NOFOLLOW) as fd: fd.read() #@
51+
with os.open("file.txt", os.O_RDONLY | os.O_NOFOLLOW, 0o644) as fd: fd.read() #@
52+
with bla.open("file.txt") as fd: fd.read() #@
53+
with bla.open("file.txt", os.O_RDONLY) as fd: fd.read() #@
54+
with bla.open("file.txt", os.O_RDONLY, 0o644) as fd: fd.read() #@
55+
with bla.open("file.txt", os.O_RDONLY, 0o755) as fd: fd.read() #@
56+
with bla.open("file.txt", os.O_RDONLY | os.O_NOFOLLOW) as fd: fd.read() #@
57+
with bla.open("file.txt", os.O_RDONLY | os.O_NOFOLLOW, 0o644) as fd: fd.read() #@
58+
with bla.open("file.txt", os.O_RDONLY | os.O_NOFOLLOW, 0o755) as fd: fd.read() #@
59+
"""
60+
)
61+
62+
self.checker.set_os_open_mode('True')
63+
assert self.checker._prefer_os_open
64+
65+
call_nodes = nodes[:14]
66+
with_nodes = nodes[14:]
67+
68+
with self.assertNoMessages():
69+
for idx, node in enumerate(call_nodes):
70+
self.checker.visit_call(node)
71+
for idx, node in enumerate(with_nodes):
72+
self.checker.visit_with(node)
73+
74+
# ==========================================================================
75+
76+
@pytest.mark.parametrize('mode', range(0o756, 0o777 + 1), ids=lambda arg: oct(arg))
77+
@pytest.mark.parametrize(
78+
'arg, expected_warning',
79+
(
80+
('False', False),
81+
('True', True),
82+
),
83+
)
84+
def test_os_open_call_default_modes(self, mode, arg, expected_warning):
85+
node = astroid.extract_node(f'os.open("file.txt", os.O_WRONLY, 0o{mode:o}) #@')
86+
self.checker.set_os_open_mode(arg)
87+
if expected_warning:
88+
with self.assertAddsMessages(pylint.testutils.Message(msg_id='os-open-unsafe-permissions', node=node)):
89+
self.checker.visit_call(node)
90+
else:
91+
with self.assertNoMessages():
92+
self.checker.visit_call(node)
93+
94+
# --------------------------------------------------------------------------
95+
96+
@pytest.mark.parametrize('mode', range(0o750, 0o761), ids=lambda arg: oct(arg))
97+
@pytest.mark.parametrize(
98+
'call_mode',
99+
(
100+
'os.open("file.txt", os.O_WRONLY, 0o{:o}) #@',
101+
'os.open("file.txt", flags=os.O_WRONLY, mode=0o{:o}) #@',
102+
),
103+
ids=('args', 'keyword'),
104+
)
105+
@pytest.mark.parametrize(
106+
'arg, expected_warning',
107+
(
108+
('False', False),
109+
('0o755,', True),
110+
),
111+
ids=('False-False', '[0o755]-True'),
112+
)
113+
def test_os_open_call(self, mode, call_mode, arg, expected_warning):
114+
node = astroid.extract_node(call_mode.format(mode))
115+
self.checker.set_os_open_mode(arg)
116+
if expected_warning and mode != 0o755:
117+
with self.assertAddsMessages(pylint.testutils.Message(msg_id='os-open-unsafe-permissions', node=node)):
118+
self.checker.visit_call(node)
119+
else:
120+
with self.assertNoMessages():
121+
self.checker.visit_call(node)
122+
123+
# ==========================================================================
124+
125+
@pytest.mark.parametrize('mode', range(0o756, 0o777 + 1), ids=lambda arg: oct(arg))
126+
@pytest.mark.parametrize(
127+
'call_mode',
128+
(
129+
'with os.open("file.txt", os.O_WRONLY, 0o{:o}) as fd: fd.read() #@',
130+
'with os.open("file.txt", flags=os.O_WRONLY, mode=0o{:o}) as fd: fd.read() #@',
131+
),
132+
ids=('args', 'keyword'),
133+
)
134+
@pytest.mark.parametrize(
135+
'arg, expected_warning',
136+
(
137+
('False', False),
138+
('True', True),
139+
),
140+
)
141+
def test_os_open_with_default_modes(self, mode, call_mode, arg, expected_warning):
142+
node = astroid.extract_node(call_mode.format(mode))
143+
self.checker.set_os_open_mode(arg)
144+
if expected_warning:
145+
with self.assertAddsMessages(pylint.testutils.Message(msg_id='os-open-unsafe-permissions', node=node)):
146+
self.checker.visit_with(node)
147+
else:
148+
with self.assertNoMessages():
149+
self.checker.visit_with(node)
150+
151+
# --------------------------------------------------------------------------
152+
153+
@pytest.mark.parametrize('mode', range(0o750, 0o761), ids=lambda arg: oct(arg))
154+
@pytest.mark.parametrize(
155+
'arg, expected_warning',
156+
(
157+
('False', False),
158+
('0o755,', True),
159+
),
160+
)
161+
def test_os_open_with(self, mode, arg, expected_warning):
162+
node = astroid.extract_node(f'with os.open("file.txt", os.O_WRONLY, 0o{mode:o}) as fd: fd.read() #@')
163+
self.checker.set_os_open_mode(arg)
164+
if expected_warning and mode != 0o755:
165+
with self.assertAddsMessages(pylint.testutils.Message(msg_id='os-open-unsafe-permissions', node=node)):
166+
self.checker.visit_with(node)
167+
else:
168+
with self.assertNoMessages():
169+
self.checker.visit_with(node)

tests/plugin_options_test.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,10 +64,9 @@ class TestSecureCodingStandardChecker(pylint.testutils.CheckerTestCase):
6464
)
6565
def test_os_open_mode_option(self, arg, prefer_os_open, allowed_modes):
6666
print(f'INFO: allowed_modes: {allowed_modes}')
67-
print(self.checker._os_open_mode_allowed)
6867
self.checker.set_os_open_mode(arg)
6968
assert self.checker._prefer_os_open == prefer_os_open
70-
assert self.checker._os_open_mode_allowed == allowed_modes
69+
assert self.checker._os_open_modes_allowed == allowed_modes
7170

7271
@pytest.mark.parametrize('arg', ('', ',', ',,', 'asd', 'a,', '493, a'))
7372
def test_os_open_mode_option_invalid(self, arg):

0 commit comments

Comments
 (0)