Feature/os chmod

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 -   Added W8016 to warn when using `os.mkdir` and `os.makedir` with unsafe permissions (UNIX-only)
 -   Added W8017 to warn when using `os.mkfifo` with unsafe permissions (UNIX-only)
 -   Added W8018 to warn when using `os.mknod` with unsafe permissions (UNIX-only)
+-   Added W8019 to warn when using `os.chmod` with unsafe permissions (all except Windows)
 
 ### Updated
 

README.md

@@ -32,6 +32,7 @@ pylint plugin that enforces some secure coding standards.
 | W8016 | Avoid using `os.mkdir` and `os.makedirs` with unsafe file permissions                                        |
 | W8017 | Avoid using `os.mkfifo` with unsafe file permissions                                                         |
 | W8018 | Avoid using `os.mknod` with unsafe file permissions                                                          |
+| W8019 | Avoid using `os.chmod` with unsafe permissions (W ^ X for group and others)                                  |
 
 
 ## Plugin configuration options

pylint_secure_coding_standard.py

@@ -15,7 +15,9 @@
 
 """Main file for the pylint_secure_coding_standard plugin."""
 
+import operator
 import platform
+import stat
 
 import astroid
 from pylint.checkers import BaseChecker
@@ -279,6 +281,93 @@ def _is_yaml_unsafe_call(node):
     return False
 
 
+# ==============================================================================
+
+_unop = {'-': operator.neg, 'not': operator.not_, '~': operator.inv}
+_binop = {
+    '+': operator.add,
+    '-': operator.sub,
+    '*': operator.mul,
+    '/': operator.truediv,
+    '//': operator.floordiv,
+    '%': operator.mod,
+    '^': operator.xor,
+    '|': operator.or_,
+    '&': operator.and_,
+}
+_chmod_known_mode_values = (
+    'S_ISUID',
+    'S_ISGID',
+    'S_ENFMT',
+    'S_ISVTX',
+    'S_IREAD',
+    'S_IWRITE',
+    'S_IEXEC',
+    'S_IRWXU',
+    'S_IRUSR',
+    'S_IWUSR',
+    'S_IXUSR',
+    'S_IRWXG',
+    'S_IRGRP',
+    'S_IWGRP',
+    'S_IXGRP',
+    'S_IRWXO',
+    'S_IROTH',
+    'S_IWOTH',
+    'S_IXOTH',
+)
+
+
+def _chmod_get_mode(node):
+    """
+    Extract the mode constant of a node.
+
+    Args:
+        node (astroid.node_classes.NodeNG): an AST node
+
+    Raises:
+        ValueError: if a node is encountered that cannot be processed
+    """
+    if isinstance(node, astroid.Name) and node.name in _chmod_known_mode_values:
+        return getattr(stat, node.name)
+    if (
+        isinstance(node, astroid.Attribute)
+        and isinstance(node.expr, astroid.Name)
+        and node.attrname in _chmod_known_mode_values
+        and node.expr.name == 'stat'
+    ):
+        return getattr(stat, node.attrname)
+    if isinstance(node, astroid.UnaryOp):
+        return _unop[node.op](_chmod_get_mode(node.operand))
+    if isinstance(node, astroid.BinOp):
+        return _binop[node.op](_chmod_get_mode(node.left), _chmod_get_mode(node.right))
+
+    raise ValueError(f'Do not know how to process node: {node.repr_tree()}')
+
+
+def _chmod_has_wx_for_go(node):
+    if platform.system() == 'Windows':
+        # On Windows, only stat.S_IREAD and stat.S_IWRITE can be used, all other bits are ignored
+        return False
+
+    try:
+        modes = None
+        if len(node.args) > 1:
+            modes = _chmod_get_mode(node.args[1])
+        elif node.keywords:
+            for keyword in node.keywords:
+                if keyword.arg == 'mode':
+                    modes = _chmod_get_mode(keyword.value)
+                    break
+    except ValueError:
+        return False
+    else:
+        if modes is None:
+            # NB: this would be from invalid code such as `os.chmod("file.txt")`
+            raise RuntimeError('Unable to extract `mode` argument from function call!')
+        return bool(modes & (stat.S_IWGRP | stat.S_IXGRP | stat.S_IWOTH | stat.S_IXOTH))
+
+
 # ==============================================================================
 
 
@@ -429,6 +518,11 @@ class SecureCodingStandardChecker(BaseChecker):  # pylint: disable=too-many-inst
             'os-mknod-unsafe-permissions',
             'Avoid using `os.mknod` with unsafe file permissions (by default 0 <= mode <= 0o755)',
         ),
+        'W8019': (
+            'Avoid using `os.chmod` with unsafe permissions (W ^ X for group and others)',
+            'os-chmod-unsafe-permissions',
+            'Avoid using `os.chmod` with unsafe file permissions (W ^ X for group and others)',
+        ),
     }
 
     def __init__(self, linter):
@@ -479,6 +573,8 @@ def visit_call(self, node):  # pylint: disable=too-many-branches
             self.add_message('avoid-marshal-load', node=node)
         elif _is_function_call(node, module='shelve', function='open'):
             self.add_message('avoid-shelve-open', node=node)
+        elif _is_function_call(node, module='os', function='chmod') and _chmod_has_wx_for_go(node):
+            self.add_message('os-chmod-unsafe-permissions', node=node)
         elif _is_unix():
             if (
                 _is_function_call(node, module='os', function=('mkdir', 'makedirs'))

tests/os_chmod_test.py

@@ -0,0 +1,170 @@
+# -*- coding: utf-8 -*-
+# Copyright 2021 Damien Nguyen
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import stat
+
+import astroid
+import pylint.testutils
+import pytest
+
+import pylint_secure_coding_standard as pylint_scs
+
+
+class TestSecureCodingStandardChecker(pylint.testutils.CheckerTestCase):
+    CHECKER_CLASS = pylint_scs.SecureCodingStandardChecker
+
+    @pytest.mark.parametrize(
+        's, expected',
+        (
+            ('S_IREAD', stat.S_IREAD),
+            ('stat.S_IREAD', stat.S_IREAD),
+            ('S_IREAD | S_IWRITE', stat.S_IREAD | stat.S_IWRITE),
+            ('stat.S_IREAD | stat.S_IWRITE', stat.S_IREAD | stat.S_IWRITE),
+            ('stat.S_IREAD | stat.S_IWRITE | S_IXUSR', stat.S_IREAD | stat.S_IWRITE | stat.S_IXUSR),
+        ),
+    )
+    def test_chmod_get_mode(self, s, expected):
+        node = astroid.extract_node(s + ' #@')
+        assert pylint_scs._chmod_get_mode(node) == expected
+
+    @pytest.mark.parametrize(
+        's',
+        (
+            'stat.ST_MODE',
+            'bla.S_IREAD',
+        ),
+    )
+    def test_chmod_get_mode_invalid(self, s):
+        node = astroid.extract_node(s + ' #@')
+        with pytest.raises(ValueError):
+            pylint_scs._chmod_get_mode(node)
+
+    @pytest.mark.parametrize(
+        's, expected',
+        (
+            ('-stat.S_IREAD', -stat.S_IREAD),
+            ('~stat.S_IREAD', ~stat.S_IREAD),
+            ('not stat.S_IREAD', not stat.S_IREAD),
+        ),
+    )
+    def test_chmod_get_mode_unop(self, s, expected):
+        node = astroid.extract_node(s + ' #@')
+        assert pylint_scs._chmod_get_mode(node) == expected
+
+    @pytest.mark.parametrize(
+        's, expected',
+        (
+            ('stat.S_IREAD + stat.S_IWRITE', stat.S_IREAD + stat.S_IWRITE),
+            ('stat.S_IREAD - stat.S_IWRITE', stat.S_IREAD - stat.S_IWRITE),
+            ('stat.S_IREAD * stat.S_IWRITE', stat.S_IREAD * stat.S_IWRITE),
+            ('stat.S_IREAD / stat.S_IWRITE', stat.S_IREAD / stat.S_IWRITE),
+            ('stat.S_IREAD // stat.S_IWRITE', stat.S_IREAD // stat.S_IWRITE),
+            ('stat.S_IREAD % stat.S_IWRITE', stat.S_IREAD % stat.S_IWRITE),
+            ('stat.S_IREAD ^ stat.S_IWRITE', stat.S_IREAD ^ stat.S_IWRITE),
+            ('stat.S_IREAD | stat.S_IWRITE', stat.S_IREAD | stat.S_IWRITE),
+            ('stat.S_IREAD & stat.S_IWRITE', stat.S_IREAD & stat.S_IWRITE),
+        ),
+    )
+    def test_chmod_get_mode_binop(self, s, expected):
+        node = astroid.extract_node(s + ' #@')
+        assert pylint_scs._chmod_get_mode(node) == expected
+
+    @pytest.mark.parametrize(
+        'platform, enabled_platform',
+        (
+            ('Linux', True),
+            ('Darwin', True),
+            ('Java', True),
+            ('Windows', False),
+        ),
+    )
+    @pytest.mark.parametrize('fname', ('"file.txt"', 'fname'))
+    @pytest.mark.parametrize('arg_type', ('', 'mode='), ids=('arg', 'keyword'))
+    @pytest.mark.parametrize(
+        'forbidden',
+        (
+            'S_IRGRP',  # NB: not actually a forbidden value, only for testing...
+            'S_IRWXG',
+            'S_IWGRP',
+            'S_IXGRP',
+            'S_IRWXO',
+            'S_IWOTH',
+            'S_IXOTH',
+        ),
+    )
+    @pytest.mark.parametrize(
+        's',
+        (
+            '',
+            'S_IREAD',
+            'S_IREAD | S_IWRITE',
+            'S_IRUSR | S_IWUSR | S_IXUSR',
+        ),
+        ids=lambda s: s if s else '<empty>',
+    )
+    def test_chmod(self, mocker, platform, enabled_platform, fname, arg_type, forbidden, s):
+        mocker.patch('platform.system', lambda: platform)
+
+        if s:
+            code = f'os.chmod({fname}, {arg_type}{s} | {forbidden}) #@'
+        else:
+            code = f'os.chmod({fname}, {arg_type} {forbidden}) #@'
+
+        print(code)
+        node = astroid.extract_node(code)
+        if enabled_platform and forbidden != 'S_IRGRP':
+            with self.assertAddsMessages(pylint.testutils.Message(msg_id='os-chmod-unsafe-permissions', node=node)):
+                self.checker.visit_call(node)
+        else:
+            with self.assertNoMessages():
+                self.checker.visit_call(node)
+
+    @pytest.mark.parametrize('platform', ('Linux', 'Darwin', 'Java', 'Windows'))
+    @pytest.mark.parametrize(
+        's',
+        (
+            'os.chmod("file.txt", stat.ST_MODE)',
+            'os.chmod("file.txt", other.S_IRWXO)',
+            'os.chmod("file.txt", mode)',
+            'os.chmod("file.txt", mode=mode)',
+        ),
+    )
+    def test_chmod_no_warning(self, mocker, platform, s):
+        mocker.patch('platform.system', lambda: platform)
+
+        node = astroid.extract_node(s)
+        with self.assertNoMessages():
+            self.checker.visit_call(node)
+
+    @pytest.mark.parametrize(
+        'platform, enabled_platform',
+        (
+            ('Linux', True),
+            ('Darwin', True),
+            ('Java', True),
+            ('Windows', False),
+        ),
+    )
+    @pytest.mark.parametrize('s', ('os.chmod("file")',))
+    def test_chmod_invalid_raise(self, mocker, platform, enabled_platform, s):
+        mocker.patch('platform.system', lambda: platform)
+
+        node = astroid.extract_node(s)
+        if enabled_platform:
+            with pytest.raises(RuntimeError):
+                self.checker.visit_call(node)
+        else:
+            with self.assertNoMessages():
+                self.checker.visit_call(node)