Skip to content

Commit 331b9b6

Browse files
TakishimaTakishima
authored
Feature/os chmod (#19)
* Implement handling of `os.chmod` * Update README * Update CHANGELOG * Improve function naming * Better processing of the mode option by evaluating its expression * Add missing logical NOT operator (~)
1 parent a321b7c commit 331b9b6

File tree

4 files changed

+268
-0
lines changed

4 files changed

+268
-0
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1212
- Added W8016 to warn when using `os.mkdir` and `os.makedir` with unsafe permissions (UNIX-only)
1313
- Added W8017 to warn when using `os.mkfifo` with unsafe permissions (UNIX-only)
1414
- Added W8018 to warn when using `os.mknod` with unsafe permissions (UNIX-only)
15+
- Added W8019 to warn when using `os.chmod` with unsafe permissions (all except Windows)
1516

1617
### Updated
1718

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ pylint plugin that enforces some secure coding standards.
3232
| W8016 | Avoid using `os.mkdir` and `os.makedirs` with unsafe file permissions |
3333
| W8017 | Avoid using `os.mkfifo` with unsafe file permissions |
3434
| W8018 | Avoid using `os.mknod` with unsafe file permissions |
35+
| W8019 | Avoid using `os.chmod` with unsafe permissions (W ^ X for group and others) |
3536

3637

3738
## Plugin configuration options

‎pylint_secure_coding_standard.py‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515

1616
"""Main file for the pylint_secure_coding_standard plugin."""
1717

18+
import operator
1819
import platform
20+
import stat
1921

2022
import astroid
2123
from pylint.checkers import BaseChecker
@@ -279,6 +281,93 @@ def _is_yaml_unsafe_call(node):
279281
return False
280282

281283

284+
# ==============================================================================
285+
286+
_unop = {'-': operator.neg, 'not': operator.not_, '~': operator.inv}
287+
_binop = {
288+
'+': operator.add,
289+
'-': operator.sub,
290+
'*': operator.mul,
291+
'/': operator.truediv,
292+
'//': operator.floordiv,
293+
'%': operator.mod,
294+
'^': operator.xor,
295+
'|': operator.or_,
296+
'&': operator.and_,
297+
}
298+
_chmod_known_mode_values = (
299+
'S_ISUID',
300+
'S_ISGID',
301+
'S_ENFMT',
302+
'S_ISVTX',
303+
'S_IREAD',
304+
'S_IWRITE',
305+
'S_IEXEC',
306+
'S_IRWXU',
307+
'S_IRUSR',
308+
'S_IWUSR',
309+
'S_IXUSR',
310+
'S_IRWXG',
311+
'S_IRGRP',
312+
'S_IWGRP',
313+
'S_IXGRP',
314+
'S_IRWXO',
315+
'S_IROTH',
316+
'S_IWOTH',
317+
'S_IXOTH',
318+
)
319+
320+
321+
def _chmod_get_mode(node):
322+
"""
323+
Extract the mode constant of a node.
324+
325+
Args:
326+
node (astroid.node_classes.NodeNG): an AST node
327+
328+
Raises:
329+
ValueError: if a node is encountered that cannot be processed
330+
"""
331+
if isinstance(node, astroid.Name) and node.name in _chmod_known_mode_values:
332+
return getattr(stat, node.name)
333+
if (
334+
isinstance(node, astroid.Attribute)
335+
and isinstance(node.expr, astroid.Name)
336+
and node.attrname in _chmod_known_mode_values
337+
and node.expr.name == 'stat'
338+
):
339+
return getattr(stat, node.attrname)
340+
if isinstance(node, astroid.UnaryOp):
341+
return _unop[node.op](_chmod_get_mode(node.operand))
342+
if isinstance(node, astroid.BinOp):
343+
return _binop[node.op](_chmod_get_mode(node.left), _chmod_get_mode(node.right))
344+
345+
raise ValueError(f'Do not know how to process node: {node.repr_tree()}')
346+
347+
348+
def _chmod_has_wx_for_go(node):
349+
if platform.system() == 'Windows':
350+
# On Windows, only stat.S_IREAD and stat.S_IWRITE can be used, all other bits are ignored
351+
return False
352+
353+
try:
354+
modes = None
355+
if len(node.args) > 1:
356+
modes = _chmod_get_mode(node.args[1])
357+
elif node.keywords:
358+
for keyword in node.keywords:
359+
if keyword.arg == 'mode':
360+
modes = _chmod_get_mode(keyword.value)
361+
break
362+
except ValueError:
363+
return False
364+
else:
365+
if modes is None:
366+
# NB: this would be from invalid code such as `os.chmod("file.txt")`
367+
raise RuntimeError('Unable to extract `mode` argument from function call!')
368+
return bool(modes & (stat.S_IWGRP | stat.S_IXGRP | stat.S_IWOTH | stat.S_IXOTH))
369+
370+
282371
# ==============================================================================
283372

284373

@@ -429,6 +518,11 @@ class SecureCodingStandardChecker(BaseChecker): # pylint: disable=too-many-inst
429518
'os-mknod-unsafe-permissions',
430519
'Avoid using `os.mknod` with unsafe file permissions (by default 0 <= mode <= 0o755)',
431520
),
521+
'W8019': (
522+
'Avoid using `os.chmod` with unsafe permissions (W ^ X for group and others)',
523+
'os-chmod-unsafe-permissions',
524+
'Avoid using `os.chmod` with unsafe file permissions (W ^ X for group and others)',
525+
),
432526
}
433527

434528
def __init__(self, linter):
@@ -479,6 +573,8 @@ def visit_call(self, node): # pylint: disable=too-many-branches
479573
self.add_message('avoid-marshal-load', node=node)
480574
elif _is_function_call(node, module='shelve', function='open'):
481575
self.add_message('avoid-shelve-open', node=node)
576+
elif _is_function_call(node, module='os', function='chmod') and _chmod_has_wx_for_go(node):
577+
self.add_message('os-chmod-unsafe-permissions', node=node)
482578
elif _is_unix():
483579
if (
484580
_is_function_call(node, module='os', function=('mkdir', 'makedirs'))

‎tests/os_chmod_test.py‎

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
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 stat
17+
18+
import astroid
19+
import pylint.testutils
20+
import pytest
21+
22+
import pylint_secure_coding_standard as pylint_scs
23+
24+
25+
class TestSecureCodingStandardChecker(pylint.testutils.CheckerTestCase):
26+
CHECKER_CLASS = pylint_scs.SecureCodingStandardChecker
27+
28+
@pytest.mark.parametrize(
29+
's, expected',
30+
(
31+
('S_IREAD', stat.S_IREAD),
32+
('stat.S_IREAD', stat.S_IREAD),
33+
('S_IREAD | S_IWRITE', stat.S_IREAD | stat.S_IWRITE),
34+
('stat.S_IREAD | stat.S_IWRITE', stat.S_IREAD | stat.S_IWRITE),
35+
('stat.S_IREAD | stat.S_IWRITE | S_IXUSR', stat.S_IREAD | stat.S_IWRITE | stat.S_IXUSR),
36+
),
37+
)
38+
def test_chmod_get_mode(self, s, expected):
39+
node = astroid.extract_node(s + ' #@')
40+
assert pylint_scs._chmod_get_mode(node) == expected
41+
42+
@pytest.mark.parametrize(
43+
's',
44+
(
45+
'stat.ST_MODE',
46+
'bla.S_IREAD',
47+
),
48+
)
49+
def test_chmod_get_mode_invalid(self, s):
50+
node = astroid.extract_node(s + ' #@')
51+
with pytest.raises(ValueError):
52+
pylint_scs._chmod_get_mode(node)
53+
54+
@pytest.mark.parametrize(
55+
's, expected',
56+
(
57+
('-stat.S_IREAD', -stat.S_IREAD),
58+
('~stat.S_IREAD', ~stat.S_IREAD),
59+
('not stat.S_IREAD', not stat.S_IREAD),
60+
),
61+
)
62+
def test_chmod_get_mode_unop(self, s, expected):
63+
node = astroid.extract_node(s + ' #@')
64+
assert pylint_scs._chmod_get_mode(node) == expected
65+
66+
@pytest.mark.parametrize(
67+
's, expected',
68+
(
69+
('stat.S_IREAD + stat.S_IWRITE', stat.S_IREAD + stat.S_IWRITE),
70+
('stat.S_IREAD - stat.S_IWRITE', stat.S_IREAD - stat.S_IWRITE),
71+
('stat.S_IREAD * stat.S_IWRITE', stat.S_IREAD * stat.S_IWRITE),
72+
('stat.S_IREAD / stat.S_IWRITE', stat.S_IREAD / stat.S_IWRITE),
73+
('stat.S_IREAD // stat.S_IWRITE', stat.S_IREAD // stat.S_IWRITE),
74+
('stat.S_IREAD % stat.S_IWRITE', stat.S_IREAD % stat.S_IWRITE),
75+
('stat.S_IREAD ^ stat.S_IWRITE', stat.S_IREAD ^ stat.S_IWRITE),
76+
('stat.S_IREAD | stat.S_IWRITE', stat.S_IREAD | stat.S_IWRITE),
77+
('stat.S_IREAD & stat.S_IWRITE', stat.S_IREAD & stat.S_IWRITE),
78+
),
79+
)
80+
def test_chmod_get_mode_binop(self, s, expected):
81+
node = astroid.extract_node(s + ' #@')
82+
assert pylint_scs._chmod_get_mode(node) == expected
83+
84+
@pytest.mark.parametrize(
85+
'platform, enabled_platform',
86+
(
87+
('Linux', True),
88+
('Darwin', True),
89+
('Java', True),
90+
('Windows', False),
91+
),
92+
)
93+
@pytest.mark.parametrize('fname', ('"file.txt"', 'fname'))
94+
@pytest.mark.parametrize('arg_type', ('', 'mode='), ids=('arg', 'keyword'))
95+
@pytest.mark.parametrize(
96+
'forbidden',
97+
(
98+
'S_IRGRP', # NB: not actually a forbidden value, only for testing...
99+
'S_IRWXG',
100+
'S_IWGRP',
101+
'S_IXGRP',
102+
'S_IRWXO',
103+
'S_IWOTH',
104+
'S_IXOTH',
105+
),
106+
)
107+
@pytest.mark.parametrize(
108+
's',
109+
(
110+
'',
111+
'S_IREAD',
112+
'S_IREAD | S_IWRITE',
113+
'S_IRUSR | S_IWUSR | S_IXUSR',
114+
),
115+
ids=lambda s: s if s else '<empty>',
116+
)
117+
def test_chmod(self, mocker, platform, enabled_platform, fname, arg_type, forbidden, s):
118+
mocker.patch('platform.system', lambda: platform)
119+
120+
if s:
121+
code = f'os.chmod({fname}, {arg_type}{s} | {forbidden}) #@'
122+
else:
123+
code = f'os.chmod({fname}, {arg_type} {forbidden}) #@'
124+
125+
print(code)
126+
node = astroid.extract_node(code)
127+
if enabled_platform and forbidden != 'S_IRGRP':
128+
with self.assertAddsMessages(pylint.testutils.Message(msg_id='os-chmod-unsafe-permissions', node=node)):
129+
self.checker.visit_call(node)
130+
else:
131+
with self.assertNoMessages():
132+
self.checker.visit_call(node)
133+
134+
@pytest.mark.parametrize('platform', ('Linux', 'Darwin', 'Java', 'Windows'))
135+
@pytest.mark.parametrize(
136+
's',
137+
(
138+
'os.chmod("file.txt", stat.ST_MODE)',
139+
'os.chmod("file.txt", other.S_IRWXO)',
140+
'os.chmod("file.txt", mode)',
141+
'os.chmod("file.txt", mode=mode)',
142+
),
143+
)
144+
def test_chmod_no_warning(self, mocker, platform, s):
145+
mocker.patch('platform.system', lambda: platform)
146+
147+
node = astroid.extract_node(s)
148+
with self.assertNoMessages():
149+
self.checker.visit_call(node)
150+
151+
@pytest.mark.parametrize(
152+
'platform, enabled_platform',
153+
(
154+
('Linux', True),
155+
('Darwin', True),
156+
('Java', True),
157+
('Windows', False),
158+
),
159+
)
160+
@pytest.mark.parametrize('s', ('os.chmod("file")',))
161+
def test_chmod_invalid_raise(self, mocker, platform, enabled_platform, s):
162+
mocker.patch('platform.system', lambda: platform)
163+
164+
node = astroid.extract_node(s)
165+
if enabled_platform:
166+
with pytest.raises(RuntimeError):
167+
self.checker.visit_call(node)
168+
else:
169+
with self.assertNoMessages():
170+
self.checker.visit_call(node)

0 commit comments

Comments
 (0)