diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c7c0ae..8dfe8c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Added E8010 to avoid using `os.popen()` as it internally uses `subprocess.Popen` with `shell=True` +- Added E8011 to avoid using `shlex.quote()` on non-POSIX platforms. ## [1.1.0] - 2021-07-02 diff --git a/README.md b/README.md index da195ee..1354380 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,7 @@ pylint plugin that enforces some secure coding standards. | C8008 | Avoid `assert` statements in production code | | R8009 | Use of builtin `open` for writing is discouraged in favor of `os.open` to allow for setting file permissions | | E8010 | Avoid using `os.popen()` as it internally uses `subprocess.Popen` with `shell=True` | +| E8011 | Use of `shlex.quote()` should be avoided on non-POSIX platforms | ## Pre-commit hook diff --git a/pylint_secure_coding_standard.py b/pylint_secure_coding_standard.py index bb01e5a..b5dd222 100644 --- a/pylint_secure_coding_standard.py +++ b/pylint_secure_coding_standard.py @@ -15,12 +15,25 @@ """Main file for the pylint_secure_coding_standard plugin""" +import platform + import astroid from pylint.checkers import BaseChecker from pylint.interfaces import IAstroidChecker +# ============================================================================== +# Helper functions + + +def _is_posix(): + """Return True if the current system is POSIX-compatible.""" + # NB: we could simply use `os.name` instead of `platform.system()`. However, that solution would be difficult to + # test using `mock` as a few modules (like `pytest`) actually use it internally... + return platform.system() in ('Linux', 'Darwin') + + # ============================================================================== @@ -171,6 +184,15 @@ def _is_jsonpickle_encode_call(node): return False +def _is_shlex_quote_call(node): + return not _is_posix() and ( + isinstance(node.func, astroid.Attribute) + and isinstance(node.func.expr, astroid.Name) + and node.func.expr.name == 'shlex' + and node.func.attrname == 'quote' + ) + + # ============================================================================== @@ -235,6 +257,11 @@ class SecureCodingStandardChecker(BaseChecker): 'avoid-os-popen', 'Use of `os.popen()` should be avoided, as it internally uses `subprocess.Popen` with `shell=True`', ), + 'E8011': ( + 'Avoid using `shlex.quote()` on non-POSIX platforms', + 'avoid-shlex-quote-on-non-posix', + 'Use of `shlex.quote()` should be avoided on non-POSIX platforms (such as Windows)', + ), } options = {} @@ -263,6 +290,8 @@ def visit_call(self, node): self.add_message('replace-builtin-open', node=node) elif isinstance(node.func, astroid.Name) and (node.func.name in ('eval', 'exec')): self.add_message('avoid-eval-exec', node=node) + elif _is_shlex_quote_call(node): + self.add_message('avoid-shlex-quote-on-non-posix', node=node) def visit_import(self, node): """ @@ -288,6 +317,8 @@ def visit_importfrom(self, node): self.add_message('avoid-os-system', node=node) elif node.modname == 'os' and [name for (name, _) in node.names if name == 'popen']: self.add_message('avoid-os-popen', node=node) + elif not _is_posix() and node.modname == 'shlex' and [name for (name, _) in node.names if name == 'quote']: + self.add_message('avoid-shlex-quote-on-non-posix', node=node) def visit_with(self, node): """ diff --git a/pyproject.toml b/pyproject.toml index 1a0ea5c..de79a18 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -66,6 +66,7 @@ build-backend = "setuptools.build_meta" minversion = '6.0' addopts = '-pno:warnings' testpaths = ['tests'] +mock_traceback_monkeypatch = false [tool.setuptools_scm] write_to = 'VERSION.txt' diff --git a/setup.cfg b/setup.cfg index 9fa076c..2620617 100644 --- a/setup.cfg +++ b/setup.cfg @@ -31,8 +31,10 @@ install_requires = [options.extras_require] test = + mock pytest pytest-cov + pytest-mock [bdist_wheel] diff --git a/tests/shlex_quote_test.py b/tests/shlex_quote_test.py new file mode 100644 index 0000000..458eb9f --- /dev/null +++ b/tests/shlex_quote_test.py @@ -0,0 +1,70 @@ +# -*- 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 astroid +import pytest + +import pylint_secure_coding_standard as pylint_scs +import pylint.testutils + + +class TestSecureCodingStandardChecker(pylint.testutils.CheckerTestCase): + CHECKER_CLASS = pylint_scs.SecureCodingStandardChecker + + @pytest.mark.parametrize( + 'platform, expected_success', + ( + ('Linux', True), + ('Darwin', True), + ('Java', False), + ('Windows', False), + ), + ) + @pytest.mark.parametrize('s', ('from shlex import quote',)) + def test_shlex_quote_importfrom(self, mocker, platform, expected_success, s): + mocker.patch('platform.system', lambda: platform) + + node = astroid.extract_node(s + ' #@') + if expected_success: + self.checker.visit_importfrom(node) + else: + with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-shlex-quote-on-non-posix', node=node)): + self.checker.visit_importfrom(node) + + @pytest.mark.parametrize( + 'platform, expected_success', + ( + ('Linux', True), + ('Darwin', True), + ('Java', False), + ('Windows', False), + ), + ) + @pytest.mark.parametrize( + 's', + ( + 'shlex.quote("ls -l")', + 'shlex.quote(command_str)', + ), + ) + def test_shlex_call_quote(self, mocker, platform, expected_success, s): + mocker.patch('platform.system', lambda: platform) + + node = astroid.extract_node(s + ' #@') + if expected_success: + self.checker.visit_call(node) + else: + with self.assertAddsMessages(pylint.testutils.Message(msg_id='avoid-shlex-quote-on-non-posix', node=node)): + self.checker.visit_call(node)